[Bug 485954] Review Request: marlin - A Sound Sample Editor for GNOME
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=485954 --- Comment #17 from Fedora Update System 2009-02-25 11:23:28 EDT --- marlin-0.13-4.fc10 has been pushed to the Fedora 10 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: marlin - A Sound Sample Editor for GNOME
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=485954 Fedora Update System changed: What|Removed |Added Fixed In Version||0.13-4.fc10 -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: marlin - A Sound Sample Editor for GNOME
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=485954 Dodji Seketeli changed: What|Removed |Added Status|ASSIGNED|CLOSED Resolution||NEXTRELEASE -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: marlin - A Sound Sample Editor for GNOME
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=485954 --- Comment #16 from Fedora Update System 2009-02-24 18:25:28 EDT --- marlin-0.13-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/marlin-0.13-4.fc10 -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: marlin - A Sound Sample Editor for GNOME
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=485954 Kevin Fenzi changed: What|Removed |Added Summary|Review Request: Marlin - A |Review Request: marlin - A |Sound Sample Editor for |Sound Sample Editor for |GNOME |GNOME Flag|fedora-cvs? |fedora-cvs+ --- Comment #15 from Kevin Fenzi 2009-02-24 16:06:02 EDT --- cvs done. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin - A Sound Sample Editor for GNOME
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=485954 Fabian Affolter changed: What|Removed |Added Summary|Review Request: Marlin, A |Review Request: Marlin - A |Sound Sample Editor for |Sound Sample Editor for |GNOME. |GNOME -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 Dodji Seketeli changed: What|Removed |Added Flag||fedora-cvs? --- Comment #14 from Dodji Seketeli 2009-02-23 04:26:21 EDT --- New Package CVS Request === Package Name: marlin Short Description: A Sound Sample Editor for GNOME Owners: dodji Branches: F-10 InitialCC: dodji -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 Joseph Smidt changed: What|Removed |Added Flag|fedora-review? |fedora-review+ --- Comment #13 from Joseph Smidt 2009-02-22 19:58:19 EDT --- Okay, looks good. 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #12 from Dodji Seketeli 2009-02-22 11:27:00 EDT --- Ooops, good catch. I didn't notice that. The problem comes from the tarball that installs the marlin icon in the wrong location. I filed a bug to upstream with a patch that fixes the problem: http://bugzilla.gnome.org/show_bug.cgi?id=572750. I added the patch to the package for now. Updated SPEC and SRPM are available at: http://dodji.fedorapeople.org/rpms/marlin/marlin-4.spec http://dodji.fedorapeople.org/rpms/marlin/marlin-0.13-4.fc10.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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #11 from Joseph Smidt 2009-02-21 13:57:39 EDT --- At first glance the packaging looks good. However, when I install and run marlin there is no icon in the menu, yet there is one called for in the .desktop file. (As there should be). -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #10 from Joseph Smidt 2009-02-21 09:13:56 EDT --- Okay, sorry for the misunderstanding about the unversioned libraries situation. I will continue 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #9 from Dodji Seketeli 2009-02-21 04:21:53 EDT --- > --- Comment #8 from Tom "spot" Callaway 2009-02-20 19:57:28 EDT --- [...] > So, unfortunately, this is a blocker. They either go in -devel or they don't > get packaged. My point was to *not* package them. I was not pushing for having the *.so in the library. The reasons I was giving were to explain why I didn't want to ship a -devel package. Not to ship *.so in the package. And as far as can tell from the spec file I pointed to at http://people.redhat.com/dseketel/rpms/marlin/marlin-3.spec, there are no unversioned library in the package. If I am missing something, please let me know. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 Tom "spot" Callaway changed: What|Removed |Added CC||tcall...@redhat.com --- Comment #8 from Tom "spot" Callaway 2009-02-20 19:57:28 EDT --- (In reply to comment #5) > > - Unversioned shared libraries should go into a -devel subpackage > > Ah, in theory yes. But I did talk with upstream about this and he > doesn't want to have a devel package yet, even though the architecture > of marlin is done so that external apps can benefit from it's internal > libraries. The reason is that the internal libraries are still a moving > target so he can't guarantee any API/ABI compatibility yet. When he can > do that, we can start shipping a -devel package I think. > > Does this make any sense ? Not really. This is a very bad idea. Upstream should be versioning any libraries that they want anyone to use. Even if the API/ABI changes aggressively, he should just bump the solib versioning aggressively. With unversioned shared libraries in the main package, any other package that uses those libraries will not know when the ABI changes have broken it. RPM will be unable to track these breaks. We put the .so files in -devel specifically to make it obvious that no normal package should depend on anything -devel. So, unfortunately, this is a blocker. They either go in -devel or they don't get packaged. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #7 from Dodji Seketeli 2009-02-19 13:01:25 EDT --- > --- Comment #6 from Joseph Smidt 2009-02-19 11:21:13 > EDT --- > Fabian, correct me if I'm wrong, but gtk2 being a BuildRequires won't actually > pull hicolor-icon-theme into Requires. Right, sorry. I miss-read what Fabian said. I thought he was saying hicolor-icon-theme was missing in Requires. My bad. I will update the package then. Were my other comments correct ? -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #6 from Joseph Smidt 2009-02-19 11:21:13 EDT --- Fabian, correct me if I'm wrong, but gtk2 being a BuildRequires won't actually pull hicolor-icon-theme into Requires. If the end user doesn't build the package himself/herself they will never receive hicolor-icon-theme. So I believe you still need that for Requires. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #5 from Dodji Seketeli 2009-02-19 10:13:40 EDT --- I have updated the spec/srpm following your comments, at: http://people.redhat.com/dseketel/rpms/marlin/marlin-3.spec http://people.redhat.com/dseketel/rpms/marlin/marlin-0.13-3.fc10.src.rpm The build results for F-10 and F-11 are at: http://koji.fedoraproject.org/koji/taskinfo?taskID=1139769 http://koji.fedoraproject.org/koji/taskinfo?taskID=1139841 > --- Comment #4 from Fabian Affolter 2009-02-19 > 03:45:30 EDT --- [...] > > - '--vendor fedora' is obsolete Right. Removed. > https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files > - Isn't 'BuildRequires: gettext' (for translation) I believe this is in the Requires of intltool that is BuildRequire'ed by Marlin already. > and 'Requires: hicolor-icon-theme' (for icons) missing? I believe this is in the Requires of gtk2 that is BuildRequired'd by Marlin already. > - Unversioned shared libraries should go into a -devel subpackage Ah, in theory yes. But I did talk with upstream about this and he doesn't want to have a devel package yet, even though the architecture of marlin is done so that external apps can benefit from it's internal libraries. The reason is that the internal libraries are still a moving target so he can't guarantee any API/ABI compatibility yet. When he can do that, we can start shipping a -devel package I think. Does this make any sense ? > - Take a look at > https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database > about 'Requires(post)/(postun): desktop-file-utils' > Okay. Thanks. I removed the 'Requires(post)/(postun): desktop-file-utils'. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 Fabian Affolter changed: What|Removed |Added CC||fab...@bernewireless.net --- Comment #4 from Fabian Affolter 2009-02-19 03:45:30 EDT --- Some other quick comments on your spec file - '--vendor fedora' is obsolete https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files - Isn't 'BuildRequires: gettext' (for translation) and 'Requires: hicolor-icon-theme' (for icons) missing? - Unversioned shared libraries should go into a -devel subpackage https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages - Take a look at https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database about 'Requires(post)/(postun): desktop-file-utils' -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 Fabian Affolter changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|nob...@fedoraproject.org|josephsm...@gmail.com -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954
--- Comment #3 from Dodji Seketeli 2009-02-18 05:21:10 EDT
---
Hello Joseph,
Thanks for the quick review. It's really appreciated.
Please find updated spec file, srpm and rpmlint output at:
http://people.redhat.com/dseketel/rpms/marlin/marlin.spec-2
http://people.redhat.com/dseketel/rpms/marlin/marlin-0.13-2.fc10.src.rpm
http://people.redhat.com/dseketel/rpms/marlin/marlin-0.13-2.rpmlint.txt
Koji tasks for the new srpm are:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1135111
http://koji.fedoraproject.org/koji/taskinfo?taskID=1135128
> --- Comment #2 from Joseph Smidt 2009-02-18
[...]
> 1. Please be consistant with macros. In some places you use $RPM_BUILD_ROOT,
> in others %{buildroot}.
Ok, only %{buildroot} is used now.
> 2. There is a Requires(pre) but no %pre section.
Ooopsy. Added.
> 3. https://fedoraproject.org/wiki/Packaging/ScriptletSnippets says
> scrollkeeper
> should look like this:
> %post
> scrollkeeper-update -q -o %{_datadir}/omf/%{name} || :
Done.
> %postun
> scrollkeeper-update -q || :
>
Done.
> And like this for update-desktop-database:
>
> %post
> update-desktop-database &> /dev/null || :
>
Done.
> %postun
> update-desktop-database &> /dev/null || :
>
Done.
> Also, you need to add:
> Requires(post): desktop-file-utils
> Requires(postun): desktop-file-utils
Done.
> 4. Remember to post your output for rpmlint.
>
Done.
For the executable warnings, I have filed upstream bug
http://bugzilla.gnome.org/show_bug.cgi?id=572255.
Thank you for your time.
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954
--- Comment #2 from Joseph Smidt 2009-02-18
01:01:29 EDT ---
Initailly a lot looks good.
A couple issues:
1. Please be consistant with macros. In some places you use $RPM_BUILD_ROOT,
in others %{buildroot}.
2. There is a Requires(pre) but no %pre section.
3. https://fedoraproject.org/wiki/Packaging/ScriptletSnippets says scrollkeeper
should look like this:
%post
scrollkeeper-update -q -o %{_datadir}/omf/%{name} || :
%postun
scrollkeeper-update -q || :
And like this for update-desktop-database:
%post
update-desktop-database &> /dev/null || :
%postun
update-desktop-database &> /dev/null || :
Also, you need to add:
Requires(post): desktop-file-utils
Requires(postun): desktop-file-utils
4. Remember to post your output for rpmlint.
Great work so far.
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 --- Comment #1 from Joseph Smidt 2009-02-17 23:21:17 EDT --- I will begin reviewing this package. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 485954] Review Request: Marlin, A Sound Sample Editor for GNOME.
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=485954 Joseph Smidt changed: What|Removed |Added CC||jsm...@fedoraproject.org Flag||fedora-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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
