[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version|snap-0.6-1.fc15 |snap-0.6-1.el6 --- Comment #38 from Fedora Update System upda...@fedoraproject.org 2012-01-05 15:33:09 EST --- snap-0.6-1.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report. -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version|snap-0.6-1.fc16 |snap-0.6-1.fc15 --- Comment #37 from Fedora Update System upda...@fedoraproject.org 2012-01-03 20:56:44 EST --- snap-0.6-1.fc15 has been pushed to the Fedora 15 stable repository. If problems still persist, please make note of it in this bug report. -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version|snap-0.5-7.fc15 |snap-0.6-1.fc16 --- Comment #36 from Fedora Update System upda...@fedoraproject.org 2012-01-03 20:56:18 EST --- snap-0.6-1.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report. -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 #33 from Fedora Update System upda...@fedoraproject.org 2011-12-20 10:50:46 EST --- snap-0.6-1.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/snap-0.6-1.fc15 -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 #35 from Fedora Update System upda...@fedoraproject.org 2011-12-20 10:51:05 EST --- snap-0.6-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/snap-0.6-1.el6 -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 #34 from Fedora Update System upda...@fedoraproject.org 2011-12-20 10:50:56 EST --- snap-0.6-1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/snap-0.6-1.fc16 -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Fixed In Version|snap-0.5-7.fc16 |snap-0.5-7.fc15 --- Comment #32 from Fedora Update System upda...@fedoraproject.org 2011-12-13 16:53:03 EST --- snap-0.5-7.fc15 has been pushed to the Fedora 15 stable repository. -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ON_QA |CLOSED Fixed In Version||snap-0.5-7.fc16 Resolution||ERRATA Last Closed||2011-12-13 16:51:38 --- Comment #31 from Fedora Update System upda...@fedoraproject.org 2011-12-13 16:51:38 EST --- snap-0.5-7.fc16 has been pushed to the Fedora 16 stable repository. -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #30 from Fedora Update System upda...@fedoraproject.org 2011-12-10 16:24:31 EST --- snap-0.5-7.el6 has been pushed to the Fedora EPEL 6 testing repository. -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Mo Morsi mmo...@redhat.com changed: What|Removed |Added Summary|Review Request: Snap A |Review Request: snap A |modular cross-platform |modular cross-platform |system backup/restore |system backup/restore |utility |utility Flag||fedora-cvs? --- Comment #25 from Mo Morsi mmo...@redhat.com 2011-12-09 06:30:51 EST --- My apologies. It is acceptable to edit the summary correct? (which I have done). I would prefer the lower case version. Thank you. New Package SCM Request === Package Name: snap Short Description: A modular cross-platform system backup/restoration utility Owners: mmorsi Branches: f15 f16 el6 InitialCC: -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 #26 from Jon Ciesla limburg...@gmail.com 2011-12-09 08:12:05 EST --- Git done (by process-git-requests). -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 #29 from Fedora Update System upda...@fedoraproject.org 2011-12-09 16:29:55 EST --- snap-0.5-7.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/snap-0.5-7.el6 -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 #27 from Fedora Update System upda...@fedoraproject.org 2011-12-09 16:29:38 EST --- snap-0.5-7.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/snap-0.5-7.fc16 -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- 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
[Bug 755890] Review Request: snap A modular cross-platform system backup/restore utility
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 #28 from Fedora Update System upda...@fedoraproject.org 2011-12-09 16:29:46 EST --- snap-0.5-7.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/snap-0.5-7.fc15 -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #16 from Mo Morsi mmo...@redhat.com 2011-12-08 06:50:24 EST ---
(In reply to comment #15)
Ack! Sorry I keep finding stuff :)
1. There's no documentation in the main package. I would include a line like
this, with our without the PDF depending on if it's relevant:
%files
%doc CHANGELOG LICENSE README docs/paper.pdf
... rest of %files section ...
Done
2. Do we need python2 if we're building with python3?
If not change your BuildRequires to:
%if 0%{?with_python3}
BuildRequires: python3-devel
%else
BuildRequires: python2-devel
%endif # if with_python3
I believe the original way is the correct way. At least every package I've seen
does not conditionalize the BR: python2-devel and it is this way in the
guidelines as well
http://fedoraproject.org/wiki/Packaging:Python
BTW also fixed the DEFAULT_SNAPFILE bug (this will be shipped in the first
release after this RPM gets into Fedora, though you can manually apply the fix
for now if you want)
https://github.com/movitto/snap/commit/7a53119b1d5bd048a4ddfbdf5bf9962f3d5699dc
New uploads:
New Spec: http://mo.morsi.org/files/rpms/snap.spec
New SRPM: http://mo.morsi.org/files/rpms/snap-0.5-6.fc15.src.rpm
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #17 from Richard Shaw hobbes1...@gmail.com 2011-12-08 09:28:50
EST ---
2. Do we need python2 if we're building with python3?
If not change your BuildRequires to:
%if 0%{?with_python3}
BuildRequires: python3-devel
%else
BuildRequires: python2-devel
%endif # if with_python3
I believe the original way is the correct way. At least every package I've
seen
does not conditionalize the BR: python2-devel and it is this way in the
guidelines as well
http://fedoraproject.org/wiki/Packaging:Python
I think the section of the python guidelines you're referring to are if you're
building for *BOTH* python2 and python3[1]. I believe in your case it's an
either/or, so it shuold be conditionalized.
Let me know if that is not the case.
BTW also fixed the DEFAULT_SNAPFILE bug (this will be shipped in the first
release after this RPM gets into Fedora, though you can manually apply the fix
for now if you want)
https://github.com/movitto/snap/commit/7a53119b1d5bd048a4ddfbdf5bf9962f3d5699dc
Is there any reason not to include this fix as a patch?
Richard
[1] http://fedoraproject.org/wiki/Packaging:Python#Building_more_than_once
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #19 from Richard Shaw hobbes1...@gmail.com 2011-12-08 11:51:06 EST --- Ok, last question I hope until I do the full guideline checks... How did you include the fix? I was looking for a Patch0: ... and %patch0 ... in the spec file but it's not there. Also, I noticed you're using github. I'm not a git expert but I've noticed other projects use the tags for released versions, so in your case I would expect to see a 0.5.0 release tagged, then you would generate a patch. For instance, if you created a tag snap-0.5.0 you could use something like: git diff -p --stat snap-0.5.0 7a53119b1d5bd048a4ddfbdf5bf9962f3d5699dc snap-snapfile.patch Of course, do it first without the so you can make sure it's what you meant. If you've already included the fix, maybe just bump the patch rev and call it 0.5.1. Then you would reset your Release: tag back to 1 (but keep all the changelog history) Richard -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #18 from Mo Morsi mmo...@redhat.com 2011-12-08 11:34:49 EST ---
(In reply to comment #17)
2. Do we need python2 if we're building with python3?
If not change your BuildRequires to:
%if 0%{?with_python3}
BuildRequires: python3-devel
%else
BuildRequires: python2-devel
%endif # if with_python3
I believe the original way is the correct way. At least every package I've
seen
does not conditionalize the BR: python2-devel and it is this way in the
guidelines as well
http://fedoraproject.org/wiki/Packaging:Python
I think the section of the python guidelines you're referring to are if you're
building for *BOTH* python2 and python3[1]. I believe in your case it's an
either/or, so it shuold be conditionalized.
Let me know if that is not the case.
Makes sense. Done
BTW also fixed the DEFAULT_SNAPFILE bug (this will be shipped in the first
release after this RPM gets into Fedora, though you can manually apply the
fix
for now if you want)
https://github.com/movitto/snap/commit/7a53119b1d5bd048a4ddfbdf5bf9962f3d5699dc
Is there any reason not to include this fix as a patch?
Included the patch in the release / new rpm
Updated submission:
New Spec: http://mo.morsi.org/files/rpms/snap.spec
New SRPM: http://mo.morsi.org/files/rpms/snap-0.5-7.fc15.src.rpm
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #20 from Mo Morsi mmo...@redhat.com 2011-12-08 15:09:56 EST --- Agreed, will tag the release once all the changes to make the package fedora compliant are said and done. Would like to hold off on tagging the release until then though (yes a little backwards than normally done, but it keeps things cleaner / simpler). In any case shouldn't be a blocker for the review. -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #21 from Richard Shaw hobbes1...@gmail.com 2011-12-08 15:20:19 EST --- Yup, not a blocker, just needed to know what your plan was. Full review to follow. Richard -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 Richard Shaw hobbes1...@gmail.com changed: What|Removed |Added Flag|fedora-review? |fedora-review+ --- Comment #22 from Richard Shaw hobbes1...@gmail.com 2011-12-08 15:32:31 EST --- +: OK -: must be fixed =: should be fixed (at your discretion) ?: Question or clairification needed N: not applicable MUST: [+] rpmlint output: Good [+] follows package naming guidelines [+] spec file base name matches package name [+] package meets the packaging guidelines [+] package uses a Fedora approved license: GPLv3 [+] license field matches the actual license: Actual source is GPLv3. [+] license file is included in %doc: LICENSE [+] spec file is in American English [+] spec file is legible [+] sources match upstream: md5sum matches (b11447108c0cc0e2eb1cd249693bcde7) [+] package builds on at least one primary arch: Tested F15 x86_64 [N] appropriate use of ExcludeArch [+] all build requirements in BuildRequires [N] spec file handles locales properly [N] ldconfig in %post and %postun [+] no bundled copies of system libraries [N] no relocatable packages [+] package owns all directories that it creates [+] no files listed twice in %files [+] proper permissions on files [+] consistent use of macros [+] code or permissible content [N] large documentation in -doc [N] no runtime dependencies in %doc [N] header files in -devel [N] static libraries in -static [N] .so in -devel [N] -devel requires main package [+] package contains no libtool archives [+] package contains a desktop file, uses desktop-file-install/validate [+] package does not own files/dirs owned by other packages [+] all filenames in UTF-8 SHOULD: [+] query upstream for license text [N] description and summary contains available translations [+] package builds in mock [+] package builds on all supported arches: Tested x86_64 [+] package functions as described: [+] sane scriptlets [+] subpackages require the main package [N] placement of pkgconfig files [N] file dependencies versus package dependencies [+] package contains man pages for binaries/scripts *** APPROVED *** -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 Mo Morsi mmo...@redhat.com changed: What|Removed |Added Flag||fedora-cvs? --- Comment #23 from Mo Morsi mmo...@redhat.com 2011-12-08 17:07:33 EST --- Great! Thanks alot for the review New Package SCM Request === Package Name: snap Short Description: A modular cross-platform system backup/restoration utility Owners: mmorsi Branches: f15 f16 el6 InitialCC: -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #24 from Jon Ciesla limburg...@gmail.com 2011-12-08 18:49:18 EST --- Summary name is Snap, SCM request name is snap, they should match. -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 Richard Shaw hobbes1...@gmail.com changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|nob...@fedoraproject.org|hobbes1...@gmail.com Flag||fedora-review? --- Comment #11 from Richard Shaw hobbes1...@gmail.com 2011-12-07 09:05:32 EST --- Yes, sorry. I got busy with family and other projects I was working on. Since T.C. hasn't taken the review officially, I'll take it. If you don't mind a swap, have a look at: https://bugzilla.redhat.com/show_bug.cgi?id=753453 It should be a very easy one. Thanks, Richard -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #13 from Richard Shaw hobbes1...@gmail.com 2011-12-07 10:35:30
EST ---
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.
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
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #12 from Richard Shaw hobbes1...@gmail.com 2011-12-07 09:43:04
EST ---
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?
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.
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
Full review to come.
Richard
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #15 from Richard Shaw hobbes1...@gmail.com 2011-12-07 17:19:33
EST ---
Ack! Sorry I keep finding stuff :)
1. There's no documentation in the main package. I would include a line like
this, with our without the PDF depending on if it's relevant:
%files
%doc CHANGELOG LICENSE README docs/paper.pdf
... rest of %files section ...
2. Do we need python2 if we're building with python3?
If not change your BuildRequires to:
%if 0%{?with_python3}
BuildRequires: python3-devel
%else
BuildRequires: python2-devel
%endif # if with_python3
Richard
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #10 from Mo Morsi mmo...@redhat.com 2011-11-30 14:41:16 EST --- ping? Do either of you guys happen to be a sponsored packager that can give the official review? Would you mind doing so, so that we can get this in. 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #9 from Richard Shaw hobbes1...@gmail.com 2011-11-23 09:23:50 EST
---
(In reply to comment #8)
I don't think I'd hold up a review for it, but it's not much work to export
a
48x48 icon (png) just to be sure :)
Done, added the 48x48 icon to the project / spec
As a side note, unless I'm missing something it doesn't seem like the icons
macros are defined in F16 and up. I had to declare them myself to get it
building against rawhide on Koji. Regardless, updated:
Interesting. I never knew there were macros for icons :) Learn something new
every day!
Though from a readability standpoint, I don't think it's any better than using
%{_datadir}/icons...
Richard
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 Richard Shaw hobbes1...@gmail.com changed: What|Removed |Added CC||hobbes1...@gmail.com --- Comment #1 from Richard Shaw hobbes1...@gmail.com 2011-11-22 10:26:42 EST --- A couple of quick comments on your spec file 1. It looks like you have mixed spaces and tabs. You can fix it manually or use something like expand 2. I've never seen a man page have a .man suffix. Should it not be snap.1? -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #2 from Mo Morsi mmo...@redhat.com 2011-11-22 12:52:53 EST --- Hey thanks for the quick feedback. Updated the spec to remove the tabs and change snap.man to snap.1 New Spec: http://mo.morsi.org/files/rpms/snap.spec New SRPM: http://mo.morsi.org/files/rpms/snap-0.5-2.fc15.src.rpm -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #3 from Richard Shaw hobbes1...@gmail.com 2011-11-22 14:40:35 EST
---
I assume that the gsnap binary is a GUI[1]?
If so you should probably add a .desktop file and icon. The guidelines cover
what to do with the desktop file but where to place icons is a little more
fuzzy :)
I prefer to use /usr/share/icons/... For instance, if you had a 48x48 and
scalable (svg) icons named gsnap-48x48.png and gsnap.svg respectively you could
install them to the following locations (in your makefile or spec):
/usr/share/icons/hicolor/48x48/apps/gsnap.png (notice I rename the file without
the size)
/usr/share/icons/hicolor/scalable/apps/gsnap.svg
Then grab them both in %files with:
%{_datadir}/icons/hicolor/*/apps/*
HTH,
Richard
[1] http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files
--
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #4 from T.C. Hollingsworth tchollingswo...@gmail.com 2011-11-22
14:55:42 EST ---
RPMLint output is not clean:
$ rpmlint ./snap-0.5-2.fc15.src.rpm
snap.src: E: summary-too-long C A modular system backup/restore utility which
uses the native package management system
Please keep lines under 40 characters.
snap.src:34: W: macro-in-comment %check
snap.src:35: W: macro-in-comment %{__python}
Please escape RPM Macros in comments (e.g. %%check)
snap.src: W: invalid-url Source0: http://mo.morsi.org/files/snap/snap-0.5.tgz
HTTP Error 404: Not Found
The Source0 URL 404s. Please fix it. (Be it server-side or in the specfile.
;-)
1 packages and 0 specfiles checked; 1 errors, 3 warnings.
$ rpmlint snap-0.5-2.fc16.noarch.rpm
snap.noarch: E: summary-too-long C A modular system backup/restore utility
which uses the native package management system
See above.
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/adapters/httpd.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/filemanager.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/metadata/service.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/snapshottarget.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/files/sapt.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/mock.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/adapters/iptables.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/packages/syum.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/config.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/packages/mock.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/repos/sapt.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/__init__.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/crypto.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/adapters/mysql.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/packages/win.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/dispatcher.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/adapters/mock.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/linuxdispatcher.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/files/win.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/metadata/sfile.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/repos/mock.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/adapters/postgresql.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/metadata/snapfile.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/exceptions.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/metadata/package.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/windowsdispatcher.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/repos/syum.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/osregistry.py 0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/files/mock.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/files/syum.py 0644L
/usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/packages/sapt.py 0644L
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #5 from Mo Morsi mmo...@redhat.com 2011-11-22 18:35:59 EST ---
Again thank both of you so much for the very quick feedback. This is my first
python package (I've got a ton of ruby ones under my belt) and my first time
packaging a graphical app so also thanks for bearing with me as I work through
the relevant guidelines.
New Spec: http://mo.morsi.org/files/rpms/snap.spec
New SRPM: http://mo.morsi.org/files/rpms/snap-0.5-3.fc15.src.rpm
(In reply to comment #3)
I assume that the gsnap binary is a GUI[1]?
Yessir
If so you should probably add a .desktop file and icon. The guidelines cover
what to do with the desktop file but where to place icons is a little more
fuzzy :)
Done
I prefer to use /usr/share/icons/... For instance, if you had a 48x48 and
scalable (svg) icons named gsnap-48x48.png and gsnap.svg respectively you
could
install them to the following locations (in your makefile or spec):
/usr/share/icons/hicolor/48x48/apps/gsnap.png (notice I rename the file
without
the size)
/usr/share/icons/hicolor/scalable/apps/gsnap.svg
Then grab them both in %files with:
%{_datadir}/icons/hicolor/*/apps/*
I just included a scalable svg icon for the time being. This is acceptable
correct?
(In reply to comment #4)
RPMLint output is not clean:
$ rpmlint ./snap-0.5-2.fc15.src.rpm
snap.src: E: summary-too-long C A modular system backup/restore utility
which uses the native package management system
Please keep lines under 40 characters.
Fixed
snap.src:34: W: macro-in-comment %check
snap.src:35: W: macro-in-comment %{__python}
Please escape RPM Macros in comments (e.g. %%check)
Fixed
snap.src: W: invalid-url Source0:
http://mo.morsi.org/files/snap/snap-0.5.tgz HTTP Error 404: Not Found
The Source0 URL 404s. Please fix it. (Be it server-side or in the specfile.
;-)
Fixed (uploaded source to there)
1 packages and 0 specfiles checked; 1 errors, 3 warnings.
$ rpmlint snap-0.5-2.fc16.noarch.rpm
snap.noarch: E: summary-too-long C A modular system backup/restore utility
which uses the native package management system
See above.
Fixed
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/backends/services/adapters/httpd.py
0644L /usr/bin/python
snap.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/snap/filemanager.py 0644L /usr/bin/python
snip
Consider removing the unnecessary shebang lines from these files.
Fixed, the shebangs have been removed
snap.noarch: W: non-conffile-in-etc /etc/snap.conf
Please mark this file with %config or %config(noreplace). See
http://fedoraproject.org/wiki/Packaging:Guidelines#Configuration_files for
more
information.
Fixed
snap.noarch: W: no-manual-page-for-binary gsnap
snap.noarch: W: no-manual-page-for-binary snaptool
You might want to rename the manpage to snaptool.1 to match the binary it
describes. gsnap is graphical and doesn't strictly require a manpage, but if
it has advanced command line options it might be worthwhile to write one
anyway.
I simply symlinked snaptool.1 to snap.1 since I want 'man snap' to work as
well. gsnap has no command line options (at least not for the time being) so
not included a man page for that.
1 packages and 0 specfiles checked; 34 errors, 3 warnings.
It seems your package is missing dependencies. gsnap fails on my system with
the following error:
Traceback (most recent call last):
File /usr/bin/gsnap, line 33, in module
from gi.repository import Gtk, Gdk, GObject
ImportError: No module named gi.repository
Ah hrm, which version of Fedora are you running against? It always worked for
me right out of the box. Are you running a KDE only environment or such?
I believe the missing dependency was pygtk2 which I've added. Feel free to
correct me if I'm wrong and I'll track it down.
Additionally, you also might want to consider packaging the Gtk interface in a
snap-gtk subpackage so command-line only users don't need to install Gtk
dependencies.
Done, the gtk bits are now shipped in a separate subpackage.
As a side note, the next couple of days is a holiday in the states so I might
not be around. If so I'll get to any more feedback as soon as I get back.
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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #6 from T.C. Hollingsworth tchollingswo...@gmail.com 2011-11-22 19:08:44 EST --- (In reply to comment #5) Please keep lines under 40 characters. Fixed Glad to see you read that as 80 characters. ;-) Sorry about that. Ah hrm, which version of Fedora are you running against? It always worked for me right out of the box. Are you running a KDE only environment or such? I believe the missing dependency was pygtk2 which I've added. Feel free to correct me if I'm wrong and I'll track it down. Fedora 16. I do run KDE, but I already had PyGTK2 installed: $ rpm -q pygtk2 pygtk2-2.24.0-3.fc15.x86_64 I think pygobject3 is what's missing: # yum whatprovides /usr/lib/python2.7/site-packages/gi/ Loaded plugins: fastestmirror, langpacks, presto, refresh-packagekit pygobject3-3.0.1-1.fc16.i686 : Python 2 bindings for GObject Introspection Repo: fedora Matched from: Filename: /usr/lib/python2.7/site-packages/gi/ pygobject3-3.0.2-1.fc16.i686 : Python 2 bindings for GObject Introspection Repo: updates Matched from: Filename: /usr/lib/python2.7/site-packages/gi/ -- 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
[Bug 755890] Review Request: Snap A modular cross-platform system backup/restore utility
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 #7 from Richard Shaw hobbes1...@gmail.com 2011-11-22 20:08:31 EST
---
(In reply to comment #5)
I prefer to use /usr/share/icons/... For instance, if you had a 48x48 and
scalable (svg) icons named gsnap-48x48.png and gsnap.svg respectively you
could
install them to the following locations (in your makefile or spec):
/usr/share/icons/hicolor/48x48/apps/gsnap.png (notice I rename the file
without
the size)
/usr/share/icons/hicolor/scalable/apps/gsnap.svg
Then grab them both in %files with:
%{_datadir}/icons/hicolor/*/apps/*
I just included a scalable svg icon for the time being. This is acceptable
correct?
I think that meets the technical requirements, but here's where the guidelines
get fuzzy. The Fedora guidelines[1] say we need to meet the freedesktop
desktop entry specs[2], which in turn say we should follow the Icon Theme
Specification[3] which says we should install a 48x48 icon as a minimum[4].
Yeah, it's convoluted :)
I don't think I'd hold up a review for it, but it's not much work to export a
48x48 icon (png) just to be sure :) Although I would be interested in the
results across supported desktops (Gnome 2, Gnome Shell, KDE, XFCE, LXDE, etc.)
with only the scalable icon.
If someone went to the trouble of testing all those configurations then I think
a valid argument can be made to install only a scalable icon.
Richard
[1] http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files
[2]
http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html
[3]
http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html
[4]
http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#install_icons
--
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
