[Bug 498218] Review Request: picturetile - Tiles a bunch of images into one large "photo wall"

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=498218





--- Comment #9 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-11-08 19:51:31 EDT ---
(In reply to comment #8)
> Orcan, thanks for your review. My responses marked with + have been
> incorporated, -, not, and ? are pending your response (not yet incorporated).
> 

You are welcome. I see that almost everything is resolved. Yet, there is a
misunderstanding that was probably my fault.

> (In reply to comment #6)
> > But this package does not even have a tarball. Is there any other examples 
> > of Fedora packages that just cut the license text from the soure file? If 
> > not, I would say it is best to obey the guideline.
> 
> ? I'm unaware of any such practices (or cases where it might be applicable for
> that matter). I preferred to have the README out of the script in a %doc file,
> and figured when I did so, I could just as well generate a COPYING. It seems
> that I either should do it this way, or drop all generated %doc files 
> entirely. Which option do you recommend?
> 

I am really not sure about this. The guideline might be interpreted either way.
Could you ask this in the fedora-packaging list?

> > ? Shall we rename the executable to just picturetile? That is the upstream
> > project name.
> 
> - I agree that an extension of .pl is not necessary, but fspot looks for an
> executable picturetile.pl in $PATH, so unless we want a symlink from
> picturetile.pl to picturetile, I suggest to drop this comment.
> 

Your choice :) I am fine either way.

> 
> > ? I am a bit reluctant about the provides:
> >    Provides:       picturetile.pl = 20050314
> > Do we really need this?
> 
> ? No, not really. I just added it in case someone decides to make a Requires 
> in fspot. I can drop this without any adverse effects, should I?
> 

I would say, drop it. One RPM package providing two different versions of the
same thing might confuse people or depsolvers.

> > ? I would go with the suggestion in comment #2 for version and release 
> > numbers. Even if upstream decides to make a 0.1 release we'll be in trouble.

> + Ok. Although the version number is now fool proof, I doubt that it will be
necessary

Oh I meant something like
Version: 0
Release: 1.20050314

I prefer that we leave the Version to upstream vendors and use our conventions
only in Release. But as you said, it is unlikely that this will cause a problem
in the future, thus I leave this up to you.


> 
> > ! ah. also it would be nice to have the source file downloaded with
> >    wget -N http://...
> > so we have the correct timestamp. 
> 
> + Agreed. Though I unfortunately need some fussing around with timestamping
> since the file has to be converted to UTF-8.
> ? I'm not quite happy with the hardcoded URL in two places though. Isn't there
> a way to refer to Source0 directly (without rpm rewriting it to the local
> SOURCES directory)?
> 

Here is the misunderstanding. No, you can't use wget in a spec file. koji has
no outside access while building a package. Besides this may be considered as a
security risk.

What I meant was roughly:
- Go to your SOURCES directory and use wget -N there, so that your original
source0 file has the correct timestamp.
- Then go to your SPECS directory and and use rpmbuild -ba (Don't use wget
inside the SPEC file)
- Now you can check your newly built rpm via
   rpm -qplv picturetile-<blah blah>.rpm 
and you will see that your /usr/bin/picturetile.pl file does have the correct
timestamp.

Sorry that I caused a confusion.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.