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=755890 --- Comment #14 from Mo Morsi <mmo...@redhat.com> 2011-12-07 16:53:19 EST --- Thanks for the review. Will try to take a look at your package in a bit. Updated Submission: New Spec: http://mo.morsi.org/files/rpms/snap.spec New SRPM: http://mo.morsi.org/files/rpms/snap-0.5-5.fc15.src.rpm Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3573274 (In reply to comment #12) > Ok, I took a look at the new spec ans srpm. I ran rpmlint on the results: > > $ rpmlint mockbuild/15/snap/*.rpm > snap.src: W: file-size-mismatch snap-0.5.tgz = 706769, > http://mo.morsi.org/files/snap/snap-0.5.tgz = 702522 > snap-gtk.noarch: W: spelling-error %description -l en_US frontend -> fronted, > front end, front-end > snap-gtk.noarch: W: spelling-error %description -l en_US snapshotter -> snaps > hotter, snaps-hotter, snapshot > snap-gtk.noarch: W: no-documentation > snap-gtk.noarch: W: no-manual-page-for-binary gsnap > 3 packages and 0 specfiles checked; 0 errors, 5 warnings. > > Since you're technically "upstream" for this I assume you can fix the > file-size-mismatch issue? > Done. Updated source uploaded and the srpm rebuilt. > The other warnings can be ignored. > > Also, I changed two things in your spec. Not functional issues but more > readability/good practices related. > > 1. Right at the beginning of %install where you create directories I changed > it > to: > %install > mkdir -p $RPM_BUILD_ROOT%{_mandir}/man1 \ > $RPM_BUILD_ROOT%{_iconsscaldir} \ > $RPM_BUILD_ROOT%{_icons48dir} > > instead of letting them wrap at 80 characters. Done > > 2. Although rpmlint didn't complain about the symbolic links for the manpage > (it will if the source path is absolute path rather than relative). It still > has a much longer path than necessary. The better practice is to change to one > of the directories (in this case the source and destination paths are the > same) > and create the link there with a minimal path, i.e.: > > pushd $RPM_BUILD_ROOT/%{_mandir}/man1 > ln -s snap.1.gz snaptool.1.gz > > which will change the result from: > -rw-r--r--. 1 build build 485 Dec 7 08:38 snap.1.gz > lrwxrwxrwx. 1 build build 29 Dec 7 08:38 snaptool.1.gz -> > /usr/share/man/man1/snap.1.gz > > to: > -rw-r--r--. 1 build build 485 Dec 7 08:38 snap.1.gz > lrwxrwxrwx. 1 build build 29 Dec 7 08:38 snaptool.1.gz -> snap.1.gz Done (In reply to comment #13) > Ok, two more things: > > 1. If python 2.7 is going to be supported you need to update the Requires for > snap-gtk to something like: > > %if 0%{?with_python3} > Requires: pygobject3 > %else > Requires: pygobject2 > %endif > > Since pygobject3 doesn't exist on F15 and even if it did it would require that > python3 be used. Done > > 2. Also, I tried running it on a remote X11 session over ssh and when I > clicked > the "Backup" button I got the following in the terminal: > > $ gsnap > Traceback (most recent call last): > File "/usr/bin/gsnap", line 83, in show_backup_window > backup_window = BackupOperationWindow() > File "/usr/bin/gsnap", line 122, in __init__ > snapfile = snap.options.DEFAULT_SNAPFILE + "-" + snapfile_id + ".tgz" > AttributeError: 'module' object has no attribute 'DEFAULT_SNAPFILE' > > Richard Ah ya this is an upstream bug. Will look into fixing for the next release. Appreciate it. -- 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. _______________________________________________ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
