[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Fedora Update System changed: What|Removed |Added Fixed In Version|yarock-0.9.64-3.fc19|yarock-0.9.64-3.fc20 --- Comment #66 from Fedora Update System --- yarock-0.9.64-3.fc20 has been pushed to the Fedora 20 stable repository. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Fedora Update System changed: What|Removed |Added Status|ON_QA |CLOSED Fixed In Version||yarock-0.9.64-3.fc19 Resolution|--- |ERRATA Last Closed||2013-12-16 18:03:52 --- Comment #65 from Fedora Update System --- yarock-0.9.64-3.fc19 has been pushed to the Fedora 19 stable repository. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Fedora Update System changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #64 from Fedora Update System --- yarock-0.9.64-3.fc20 has been pushed to the Fedora 20 testing repository. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #63 from Fedora Update System --- yarock-0.9.64-3.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/yarock-0.9.64-3.fc20 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #62 from Fedora Update System --- yarock-0.9.64-3.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/yarock-0.9.64-3.fc19 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Fedora Update System changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #61 from Jon Ciesla --- Git done (by process-git-requests). -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Jon Ciesla changed: What|Removed |Added Flags|fedora-cvs? |fedora-cvs+ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 James Abtahi changed: What|Removed |Added Flags||fedora-cvs? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #60 from James Abtahi --- New Package SCM Request === Package Name: yarock Short Description: A lightweight, beautiful music player Owners: jam3s Branches: f19 f20 InitialCC: -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #59 from James Abtahi ---
> Please add:
> %dir %{_datadir}/%{name}
> which is otherwise unowned. Other than that, it looks fine.
Done. I'm going to put a request for creation of a repository in the next few
hours. In the meantime please let me know of any final thoughts.
updated SPEC:
http://jam3s.fedorapeople.org/yarock.spec
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #58 from Kevin Kofler ---
Please add:
%dir %{_datadir}/%{name}
which is otherwise unowned. Other than that, it looks fine.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #57 from James Abtahi ---
I've applied Kevin Kofler's solution and now the translation files are
installed in %{_datadir}/%{name}/translations (see the installationpath patch).
I've also noticed that "qt" package translation files are installed in a
similar fashion. So this should be fine. Let me know what you think.
updated SPEC and SRPM:
http://jam3s.fedorapeople.org/yarock.spec
http://jam3s.fedorapeople.org/yarock-0.9.64-3.fc20.src.rpm
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #56 from James Abtahi --- nah, something stupid happened there. nothing related to the package. the .qt files are installed in the /usr/share/locale/yarock. Ignore my previous comment. sorry about it. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #55 from James Abtahi ---
hum... something is terribly wrong. according to this source:
http://www.cmake.org/Wiki/CMake:How_To_Build_Qt4_Software#Translations
.ts file are just the xml source translations while the .qm files are the
binary translations which are executed. but the .ts files are installed
directly in the /usr/share/locale/yarock after installation. No .qm file is
there just .ts files.
I think the original author made some mistakes in the src/CMakeLists and the
patch (.installationpath) is really building upon that bad design. Specifically
It doesn't seem right to hard-code /usr/share/locale there. One probably should
use something like this:
install(FILES ${QM_FILES} DESTINATION ${CMAKE_INSTALL_PREFIX}/translations)
but I'm no expert with these translation stuff. However, I've got a feeling
that the source of the problem is in src/CMakeLists. any ideas?
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #54 from James Abtahi ---
> Where is your latest src.rpm?
> does not match the src.rpm under http://jam3s.fedorapeople.org/
updated now:
http://jam3s.fedorapeople.org/yarock-0.9.64-2.fc20.src.rpm
> One solution that Qt programs often use is to install the *.qm files under %
> {_datadir}/%{name}/translations. It's also not really optimal, but at least it
> doesn't abuse %{_datadir}/locale.
sounds like a good solution. @Ralf Corsepius: Would this solution be alright
with you?
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #53 from Kevin Kofler ---
One solution that Qt programs often use is to install the *.qm files under
%{_datadir}/%{name}/translations. It's also not really optimal, but at least it
doesn't abuse %{_datadir}/locale.
The ideal solution would of course be to just use gettext, but unfortunately,
Qt suffers from NIH syndrome there. :-( (And KDE actually ignores that and uses
gettext, but non-KDE Qt stuff normally uses the Qt system.)
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #52 from Ralf Corsepius --- Where is your latest src.rpm? http://jam3s.fedorapeople.org/yarock.spec does not match the src.rpm under http://jam3s.fedorapeople.org/ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #51 from Ralf Corsepius ---
(In reply to James Abtahi from comment #50)
> > /usr/share/locale/yarock is a bug. Package simply MUST not install i18n
> > files there in.
>
> reference?
This is common sense ever since i18n exists and is hard-wired into many tools.
> I thought Kevin Kofler said:
>
> > The gettext scheme is /usr/share/locale/%{locale}/%{name}.mo, but Qt
> > doesn't
> > support that scheme, it expects *_%{locale}.qm (or just %{locale}.qm, which
> > some apps try to use and which is NOT supported by %find_lang) files in a
> > common directory.
> -
>
> > This package is broken and should to be fixed.
>
> and what is your solution exactly?
I haven't had a deepper look into this package yet.
I only stumbled over the
%dir %{datadir}/locale/%{name}
inside of your *spec. A line which is multiply questionable.
> I thought this was a a bug with qt.
I can't tell without having had a deeper look into your package. My guess would
be, this is an bug inside of this package.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #50 from James Abtahi ---
> /usr/share/locale/yarock is a bug. Package simply MUST not install i18n files
> there in.
reference?
I thought Kevin Kofler said:
> The gettext scheme is /usr/share/locale/%{locale}/%{name}.mo, but Qt doesn't
> support that scheme, it expects *_%{locale}.qm (or just %{locale}.qm, which
> some apps try to use and which is NOT supported by %find_lang) files in a
> common directory.
-
> This package is broken and should to be fixed.
and what is your solution exactly?
I thought this was a a bug with qt.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
Ralf Corsepius changed:
What|Removed |Added
CC||rc040...@freenet.de
--- Comment #49 from Ralf Corsepius ---
(In reply to Kevin Kofler from comment #44)
> > 1. /usr/share/locale/yarock doesn't seem to be owned. Not sure if this
> > is a find_lang bug or what.
/usr/share/locale/yarock is a bug. Package simply MUST not install i18n files
there in.
The are supposed to install them into /usr/share/locale/
> This is the usual problem with Qt translation files (used by Qt-only stuff,
> KDE uses gettext instead). There is really no standard location where to put
> them. /usr/share/locale/%{name} is not that great a location for
> translations, "yarock" is not a locale.
Exactly - This package is broken and should to be fixed.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #48 from James Abtahi --- > Usually when adding comments for patches, thats done at the top where the > patch > is defined instead of in the prep where it's applied. You might move those > comments up to the top, you can do that before you import it... Done. > I don't see any further blockers, so this package is APPROVED. Thank you very much. It's kind of exciting to see my first package approved ;) Special thanks to those who commented on and reviewed this packages: Kevin Fenzi, Rex Dieter, Kevin Kofler, Terje Røsten, and Christopher Meng. I'll proceed with the instructions indicated in the following wiki: https://fedoraproject.org/wiki/Package_SCM_admin_requests updated SPEC: http://jam3s.fedorapeople.org/yarock.spec -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #47 from Kevin Kofler --- > Usually when adding comments for patches, thats done at the top where the > patch > is defined instead of in the prep where it's applied. Well, I've seen both variants, but yes, doing it at the top is more common. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Kevin Fenzi changed: What|Removed |Added Flags|fedora-review? |fedora-review+ --- Comment #46 from Kevin Fenzi --- Great. Thanks for the pointer on the locale dir Kevin Koffler. Usually when adding comments for patches, thats done at the top where the patch is defined instead of in the prep where it's applied. You might move those comments up to the top, you can do that before you import it... I don't see any further blockers, so this package is APPROVED. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #45 from James Abtahi ---
> In any case, the directory containing the translations needs to be listed in
> %files with a %dir tag, e.g.:
> %dir %{_datadir}/locale/%{name}/
Thanks a lot for this solution. Saved me from a lot of headaches. I've added it
to the new spec.
> 2. Whats the status of upstreaming the patches? It's usually good
> practice to add comments next to any patches in the spec with links
> to any upstream bug reports, or even just a 'sent to upstream on -mm-dd'
Done. I sent the patches to upstream and added comments to the spec file. I
also fixed that little "coverart" typo in description.
updated SPEC:
http://jam3s.fedorapeople.org/yarock.spec
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #44 from Kevin Kofler ---
> 1. /usr/share/locale/yarock doesn't seem to be owned. Not sure if this
> is a find_lang bug or what. Perhaps the arguments are confusing it?
> You can manually run the find_lang.sh rpm uses and see if you can
> track it down. Or perhaps someone else has seen this?
This is the usual problem with Qt translation files (used by Qt-only stuff, KDE
uses gettext instead). There is really no standard location where to put them.
/usr/share/locale/%{name} is not that great a location for translations,
"yarock" is not a locale. The gettext scheme is
/usr/share/locale/%{locale}/%{name}.mo, but Qt doesn't support that scheme, it
expects *_%{locale}.qm (or just %{locale}.qm, which some apps try to use and
which is NOT supported by %find_lang) files in a common directory.
In any case, the directory containing the translations needs to be listed in
%files with a %dir tag, e.g.:
%dir %{_datadir}/locale/%{name}/
The %find_lang script will only collect the files inside the directory, so the
directory itself needs to be listed explicitly. And %dir means that we only
want the directory and not its contents, because the contents are found and
tagged with the correct %lang(*) tags by %find_lang.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #43 from Kevin Fenzi ---
1. /usr/share/locale/yarock doesn't seem to be owned. Not sure if this
is a find_lang bug or what. Perhaps the arguments are confusing it?
You can manually run the find_lang.sh rpm uses and see if you can
track it down. Or perhaps someone else has seen this?
2. Whats the status of upstreaming the patches? It's usually good
practice to add comments next to any patches in the spec with links
to any upstream bug reports, or even just a 'sent to upstream on -mm-dd'
Otherwise things look good from here. Solve those and I can approve. ;)
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed
= MUST items =
C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
"GPL (v2 or later)", "Unknown or generated". 5 files have unknown
license. Detailed output of licensecheck in /home/fedora/kevin/yarock
/review-yarock/licensecheck.txt
[!]: Package requires other packages for directories it uses.
Note: No known owner of /usr/share/locale/yarock
[!]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/locale/yarock
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
contains icons.
Note: icons in yarock
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is known to not require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s)
for the package is included in %doc.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or desktop-
file-validate if there is such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
= SHOULD items =
Generic:
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[!]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[x]: Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
architectures.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Ve
[Bug 1032108] Review Request: yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Kevin Fenzi changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|nob...@fedoraproject.org|ke...@scrye.com Summary|Review Request: Yarock - A |Review Request: yarock - A |Lightweight and Beautiful |Lightweight and Beautiful |Music Player|Music Player Flags||fedora-review? --- Comment #42 from Kevin Fenzi --- I'll look at reviewing this today... hopefully look for a full review later. ;) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #41 from James Abtahi --- > I agree with Rex Dieter (comment #11) there. Enumerating every single icon > (and > even every single size!) explicitly is totally pointless. It will just fail > the > build for no reason when upstream adds new icons. > I disagree with Terje's comment #12 here Now the vote is 2 to 1 in favor of using globs. In that case, We revert to globs. Since Kevin Fenzi is busy at the moment, I just hope that this package find a reviewer as soon as possible. Update SPEC: http://jam3s.fedorapeople.org/yarock.spec -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #40 from Kevin Kofler ---
> %{_datadir}/icons/hicolor/16x16/apps/application-x-yarock.png
> %{_datadir}/icons/hicolor/32x32/apps/application-x-yarock.png
> %{_datadir}/icons/hicolor/48x48/apps/application-x-yarock.png
> %{_datadir}/icons/hicolor/64x64/apps/application-x-yarock.png
> %{_datadir}/icons/hicolor/128x128/apps/application-x-yarock.png
I agree with Rex Dieter (comment #11) there. Enumerating every single icon (and
even every single size!) explicitly is totally pointless. It will just fail the
build for no reason when upstream adds new icons.
I disagree with Terje's comment #12 here:
> I want control over shipped files, changes should trigger a failed build.
>
> Control is more important than convenience for the packager.
It really makes no sense to fail the build on new added icons. It is normal for
applications to ship multiple icons: an application icon, a MIME type icon,
custom actions not covered by the icon themes etc. There is no reason to
require manually editing the specfile each time one is added.
The use of globs is at the packager's discretion. As long as the directory
ownership is correct (i.e. you MUST NOT use something like /* which ends up
owning /usr, of course!), there is no guideline that forbids using globs.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #39 from Kevin Fenzi --- I'd be happy to review this, but not sure when I will get time to do so. :( So, if anyone else would like to, please feel free... if not, I will try and do so as time permits. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #38 from Michael Schwendt --- > Shouldn't the sponsor be also the reviewer? It's somewhat a grey area, since you've been sponsored prior to your first package review request already (for becoming a co-maintainer). The following page, http://fedoraproject.org/wiki/Package_Review_Process#Reviewer only says: | The Reviewer can be any Fedora account holder, who is a member of | the packager group. There is one exception: If it is the first package | of a Contributor, the Reviewer must be in the Sponsor group and be | willing to sponsor that Contributor. That refers to review requests with the NEEDSPONSOR flag, where a sponsor is needed for real progress. But this is not your "first package", only your first package review request. Any reviewer could do the review, since you are sponsored already. For the initial guidance, it would be fair if Kevin did the first review, however. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #37 from James Abtahi --- > In actual review process has not started yet, no reviewer is assigned. Shouldn't the sponsor be also the reviewer? Kevin Fenzi sponsored me (for co-maintaining another package) into the fedora packager git commit group: > I've sponsored james to help co-maintain a infrastructure package. Removing > the NEEDSPONSOR here. Is he also responsible for reviewing this package too? I'll have to talk to him. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #36 from Terje Røsten --- In actual review process has not started yet, no reviewer is assigned. See: http://fedoraproject.org/wiki/Package_Review_Process#Review_Process for details. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #35 from James Abtahi --- If all the reviewers are satisfied with this package, please APPROVE it or otherwise let me know of any suggestions. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #34 from James Abtahi --- > fwiw, I can supply a separate (upstreamable) patch to fix build problems using > the bundled code when building with -DBUILD_SHARED_LIBS:BOOL=ON option too. You can contact the original developer 'Sebastien Amardeilh' regarding that. I've already informed him that we are packaging yarock for fedora. Anyway, Thanks for providing those fine patches. Is there any other issues that needs to be solved? Any other comments regarding the SPEC? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #33 from Rex Dieter --- fwiw, I can supply a separate (upstreamable) patch to fix build problems using the bundled code when building with -DBUILD_SHARED_LIBS:BOOL=ON option too. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #32 from James Abtahi --- > Still bad: > http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage Done, please see the %check section: http://jam3s.fedorapeople.org/yarock.spec -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #31 from Christopher Meng --- Still bad: http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #30 from James Abtahi --- Success! Actually Rex forgot to add "qtsinglecoreapplication-devel" in the build requirements. With that in place, the build is successful. updated SPEC: http://jam3s.fedorapeople.org/yarock.spec updated Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6215874 Special thanks to Rex for his patches. I think this is ready for inclusion in fedora repos. Comments are welcome. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #29 from James Abtahi --- I think I have found the solution. Rex forgot one of the dependencies. lets see... > Bundled code is never allowed in Fedora. Or we won't approve your package. > Reference: > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries Thanks for the heads up. With Rex's solution we shouldn't worry about that ;) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #28 from Terje Røsten --- Reference: https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries Or you can start here: https://fedoraproject.org/wiki/Category:Package_Maintainers#Packaging_Guidelines -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #27 from Christopher Meng --- Bundled code is never allowed in Fedora. Or we won't approve your package. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #26 from James Abtahi --- > There is a minor typo in your spec: > qtsingleappliation-devel => qtsingleapplciation-devel I meant "qtsingleapplication-devel". ;) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #25 from James Abtahi --- > See http://rdieter.fedorapeople.org/rpms/yarock/ There is a minor typo in your spec: qtsingleappliation-devel => qtsingleapplciation-devel Anyway, even after fixing that the build fails. The variable QTSINGLECOREAPPLICATION_LIBRARIES is set to NOTFOUND: http://paste.fedoraproject.org/56249/38519578 Besides, does it really worth it to add 2 more build dependencies and another patch just to make %cmake work? Wouldn't be easier to go along with my innocent cmake approach? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #24 from Rex Dieter --- See http://rdieter.fedorapeople.org/rpms/yarock/ yarock.spec : updated spec file spec.patch : patch to original .spec Yarock_0.9.64_11_source-system_libs.patch : patch applied in yarock.spec allowing use of system qtsingleappliation, qxt libraries -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #23 from Rex Dieter --- prepping patch to fix this... -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 --- Comment #22 from Rex Dieter --- I guess I'm just annoyed enough to figure out why %cmake fails. :) Ah, pretty easy to spot (2 problems actually): 1. it bundles qtsingleapplication 2. this bundled build fails when building with -DBUILD_SHARED_LIBS:BOOL=ON -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108
--- Comment #21 from James Abtahi ---
> Indeed, it's mandatory, build must use these flags. This is important for
> security and creation for debuginfo package, abrt and retrace support etc.
Done. I've added %{optflags} to CFLAGS and CPPFLAGS.
> Tip: if you get %cmake working all this will work out of box.
I wish it would work but It seems that some of the default options indicated in
(rpm --eval %cmake) is giving "make" a hard time. It's been a long time that
I've given up on %cmake.
Please check the results:
updated SPEC:
http://jam3s.fedorapeople.org/yarock.spec
updated koji build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=6214507
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1032108] Review Request: Yarock - A Lightweight and Beautiful Music Player
https://bugzilla.redhat.com/show_bug.cgi?id=1032108 Terje Røsten changed: What|Removed |Added Summary|Review Request: Yarock - A |Review Request: Yarock - A |Lightweight and Beautiful |Lightweight and Beautiful |Qt Music Player |Music Player -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
