Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: diffuse - graphical diff tool (first package, seeking sponser)
https://bugzilla.redhat.com/show_bug.cgi?id=480859
Summary: Review Request: diffuse - graphical diff tool (first package, seeking sponser) Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: fedora@coralbark.net QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://coralbark.net/fedora/diffuse/diffuse.spec SRPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-1.fc10.src.rpm RPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-1.fc10.noarch.rpm
Description:
Diffuse is a graphical tool for merging and comparing text files. Diffuse is able to compare an arbitrary number of files side-by-side and gives users the ability to manually adjust line matching and directly edit files. Diffuse can also retrieve revisions of files from Bazaar, CVS, Darcs, Git, Mercurial, Monotone, Subversion, and SVK repositories for comparison and merging.
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=480859
François Kooman fkooman@tuxed.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fkooman@tuxed.net
--- Comment #1 from François Kooman fkooman@tuxed.net 2009-01-21 09:38:38 EDT --- (Just some remarks from quickly looking over the package)
- For your Source line see: https://fedoraproject.org/wiki/PackagingDrafts/SourceUrl, maybe also make it Source0 instead of just Source.
- See https://fedoraproject.org/wiki/Packaging:Python for specific Python packaging guidelines
- See https://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo for a template of a spec file (or use rpmdev-newspec), it might be nice to create your spec file based on this or modify yours to better match the template.
rpmlint output:
[fkooman@franek SPECS]$ rpmlint diffuse.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. [fkooman@franek SPECS]$
[fkooman@franek SRPMS]$ rpmlint diffuse-0.2.15-1.fc10.src.rpm diffuse.src: W: summary-not-capitalized graphical tool for comparing and merging text files diffuse.src: W: non-standard-group Development/Tools/Version Control diffuse.src: W: no-url-tag 1 packages and 0 specfiles checked; 0 errors, 3 warnings. [fkooman@franek SRPMS]$
[fkooman@franek noarch]$ rpmlint diffuse-0.2.15-1.fc10.noarch.rpm diffuse.noarch: W: summary-not-capitalized graphical tool for comparing and merging text files diffuse.noarch: W: non-standard-group Development/Tools/Version Control diffuse.noarch: W: no-url-tag diffuse.noarch: W: conffile-without-noreplace-flag /etc/diffuserc diffuse.noarch: E: invalid-desktopfile /usr/share/applications/diffuse.desktop 1 packages and 0 specfiles checked; 1 errors, 4 warnings. [fkooman@franek noarch]$
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=480859
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |itamar@ispbrasil.com.br Depends on| |177841
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=480859
--- Comment #2 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-01-21 10:11:39 EDT --- please don't gzip man pages.
gzip -9 %{buildroot}/usr/share/man/man1/diffuse.1
all files in /usr/share/man are magically gziped by rpm
please macronify your %files section
please replace all hardcoded paths below with macros.
/usr/bin/diffuse /usr/share/diffuse/ /usr/share/applications/diffuse.desktop /usr/share/gnome/help/diffuse/C/diffuse.xml /usr/share/man/man1/diffuse.1.gz /usr/share/omf/diffuse/diffuse-C.omf /usr/share/pixmaps/diffuse.png
for more info about macros please l@@k
https://fedoraproject.org/wiki/Packaging/RPMMacros
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=480859
Miroslav Lichvar mlichvar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mlichvar@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=480859
manuel wolfshant wolfy@nobugconsulting.ro changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: diffuse - |Review Request: diffuse - |graphical diff tool (first |graphical diff tool |package, seeking sponser) | Alias| |diffuse
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=480859
--- Comment #3 from Jon Levell fedora@coralbark.net 2009-01-21 18:25:05 EDT --- I hope I've addressed all the comments. Updated files are: Spec URL: http://coralbark.net/fedora/diffuse/diffuse.spec SRPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-2.fc10.src.rpm RPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-2.fc10.noarch.rpm
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=480859
Jon Levell fedora@coralbark.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 Depends on|177841 |
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=480859
--- Comment #4 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-01-21 18:34:46 EDT --- you're missing somethint in changelog
* Wed Jan 21 2009 Jon Levell fedora@coralbark.net 0.2.15-2
should be something like this * Wed Jan 21 2009 Jon Levell fedora@coralbark.net - 0.2.15-2
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=480859
--- Comment #5 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-01-21 18:36:55 EDT --- macronify more.
from %{_bindir}/diffuse %{_datadir}/diffuse/
to %{_bindir}/%{name} %{_datadir}/%{name}/
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=480859
--- Comment #6 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-01-21 18:38:41 EDT --- change from
%defattr(-,root,root)
to %defattr(-,root,root,-)
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=480859
--- Comment #7 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2009-01-21 18:41:01 EDT --- fix your buildroot according with guidelines
https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
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=480859
--- Comment #8 from Jon Levell fedora@coralbark.net 2009-01-24 06:03:38 EDT --- Another update: Spec URL: http://coralbark.net/fedora/diffuse/diffuse.spec SRPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-3.fc10.src.rpm RPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-3.fc10.noarch.rpm
I've addressed all the comments except for #5. I considered what you suggest when making the first "macronified" version but it seems funny to change: %{_bindir}/diffuse to %{_bindir}/%{name}
If I'm not changing: %{_sysconfdir}/diffuserc to %{_sysconfdir}/%{name}rc
and I've not keen on making the second change - if we were changing the name of the program, there would be bigger issues than updating the files section.
If people feel strongly that I should make the suggested change I will. If so, should I change the diffuserc as well as: %{_bindir}/diffuse %{_datadir}/diffuse/
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=480859
Michael Schwendt bugs.michael@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |bugs.michael@gmx.net AssignedTo|nobody@fedoraproject.org |bugs.michael@gmx.net Flag| |fedora-review?
--- Comment #9 from Michael Schwendt bugs.michael@gmx.net 2009-02-07 16:43:13 EDT ---
you're missing somethint in changelog
No. The first %changelog entry quoted in comment 4 is fine.
macronify more.
No. The request in comment 5 leads to macro-madness. Only in some cases it is convenient to replace the program name with %{name} -- e.g. if you want to reuse a spec file for other packages. It doesn't happen often that a program changes its name frequently, so that using %name everywhere (even in URL and %files lists) would be justified.
* The guidelines want the .desktop file to be validated in the %install section: https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usa...
* The guidelines include specific shell code for scrollkeeper-update and desktop-data-base usage:
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Scrollkeeper https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database
* Several directories are not included in %files:
%dir %{_datadir}/gnome/help/diffuse %dir %{_datadir}/gnome/help/diffuse/C %dir %{_datadir}/omf/diffuse
https://fedoraproject.org/wiki/Packaging/UnownedDirectories
* Rest is good.
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=480859
--- Comment #10 from Jon Levell fedora@coralbark.net 2009-02-11 15:54:38 EDT --- Another update:
Spec URL: http://coralbark.net/fedora/diffuse/diffuse.spec SRPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-4.fc10.src.rpm RPM URL: http://coralbark.net/fedora/diffuse/diffuse-0.2.15-4.fc10.noarch.rpm
I've addressed the points from comment #9. I can see that I was very naive when I initially submitted the spec file on the grounds that it seemed to work fine on the systems I installed it on.
Thanks Michael (and everyone else) for the helpful comments
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=480859
Michael Schwendt bugs.michael@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #11 from Michael Schwendt bugs.michael@gmx.net 2009-02-14 13:52:57 EDT --- Yes, lots of package spec files "out there" seem to work, but there are many packaging pitfalls, and suddenly the packages break in unexpected ways. ;-)
[...]
The packaging is fine now.
diffuse-0.2.15-4.fc10.src.rpm : APPROVED
[...]
I see you don't have any other review requests currently, and the NEEDSPONSOR keyword is set. What are your plans with regard to https://fedoraproject.org/wiki/PackageMaintainers/Join ?
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=480859
--- Comment #12 from Jon Levell fedora@coralbark.net 2009-02-21 16:28:11 EDT --- Re: Comment #11
I've been giving some thought to my sponsorship request. I would like to be sponsored but I don't (at least initially) want to maintain many packages.
On the other hand, this package isn't very good evidence that I'm competent. Would a number of informal package reviews be enough?
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=480859
--- Comment #13 from Michael Schwendt bugs.michael@gmx.net 2009-02-21 16:55:51 EDT --- Yes, good idea.
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=480859
Michael Schwendt bugs.michael@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 |
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=480859
Jon Levell fedora@coralbark.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #14 from Jon Levell fedora@coralbark.net 2009-02-26 16:53:43 EDT --- New Package CVS Request ======================= Package Name: diffuse Short Description: Graphical tool for comparing and merging text files Owners: jonquark Branches: F-9 F-10 EL-5 InitialCC:
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=480859
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #15 from Kevin Fenzi kevin@tummy.com 2009-02-26 19:12:28 EDT --- cvs done.
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=480859
--- Comment #16 from Fedora Update System updates@fedoraproject.org 2009-03-01 05:44:54 EDT --- diffuse-0.2.15-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/diffuse-0.2.15-4.fc10
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=480859
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |ON_QA
--- Comment #17 from Fedora Update System updates@fedoraproject.org 2009-03-04 11:25:17 EDT --- diffuse-0.2.15-4.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update diffuse'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2310
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=480859
--- Comment #18 from Fedora Update System updates@fedoraproject.org 2009-03-11 13:59:33 EDT --- diffuse-0.2.15-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
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=480859
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |0.2.15-4.fc10 Resolution| |NEXTRELEASE
package-review@lists.fedoraproject.org