Spec URL: http://elad.fedorapeople.org/reviews/gnome-maps/gnome-maps.spec SRPM URL: http://elad.fedorapeople.org/reviews/gnome-maps/gnome-music-3.9.90-1.fc21.src.rpm Description: a simple map application for GNOME desktop Fedora Account System Username: elad Note: has a runtime requirement on geoclue2 which is still waiting for review (bug #999153)
1. Remove rm -rf $RPM_BUILD_ROOT. 2. Add icon cache refresh script for hicolor dir.
oops. Also forgot the glib schema compile scripts... Fixed. SRPM URL: http://elad.fedorapeople.org/reviews/gnome-maps/gnome-maps-3.9.90.1-2.fc20.src.rpm
Updated to latest upstream, added BuildArch: noarch SRPM URL: http://elad.fedoraproject.org/reviews/gnome-maps/gnome-maps-3.9.90.2-1.fc20.src.rpm
Could you please change the Summary to 'A map application for GNOME 3' or simply 'A map application for GNOME'?
It's widely accepted practise to omit leading articles, such as "An" and "A" to shorten summaries even further: Summary: Map application for GNOME That also looks better when displayed by package installer tools. https://fedoraproject.org/wiki/Examples_of_good_package_summaries [...] gnome-maps.src: W: summary-not-capitalized C simple map application for GNOME desktop [...] SRPM URL is invalid. s/fedoraproject/fedorapeople/ -> http://elad.fedorapeople.org/reviews/gnome-maps/gnome-maps-3.9.90.2-1.fc20.src.rpm [...] Run rpmlint (or rpmlint -i for more helpful output) on the src.rpm and all built rpms. Feel free to ignore obvious false positives in the report, but fix anything else. Preferably add a comment here about whether/when you think what rpmlint reports is correct or incorrect. [...] > Requires: geoclue2 > Requires: geocode-glib https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
(In reply to Michael Schwendt from comment #5) > [...] > > gnome-maps.src: W: summary-not-capitalized C simple map application for > GNOME desktop Will fix soon > > [...] > > SRPM URL is invalid. s/fedoraproject/fedorapeople/ -> > http://elad.fedorapeople.org/reviews/gnome-maps/gnome-maps-3.9.90.2-1.fc20. > src.rpm > Oops, yeah, I should read what I write before I click save > [...] > > Run rpmlint (or rpmlint -i for more helpful output) on the src.rpm and all > built rpms. Feel free to ignore obvious false positives in the report, but > fix > anything else. Preferably add a comment here about whether/when you think > what > rpmlint reports is correct or incorrect. > > [...] > > > Requires: geoclue2 > > Requires: geocode-glib > > https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires Since both geoclue2 and geocode-glib are new packages in Fedora, I feel like adding a version makes little to no sense - nobody will have an older version of these packages simply because older versions never existed within Fedora.
The important detail of that guideline is to add a comment that explains why the explicit Requires is needed. For example, and since it's package names which are required, sum up what inside these packages is needed.
the explicit requires are needed because rpm doesn't have a mechanism to find requirements of JS applications automatically, this would be the same for Python apps, and if you look on other python or js packages in fedora you will find explicit requires without explanatory comments. also, of course it's the package names that are listed, file-based requires are discoureged. I'm not going to add useless comments to the spec.
Of course they aren't added automatically, or else you would not add them explicitly. ;-) However, Python packages in Fedora have "python" in their name (or "py"), so a dependency of one Python based package on another Python based package is self-explaining (a comment "we need this" would be superfluous indeed). On the contrary, these two package names don't give a hint about what they may contain and whether they include JavaScript files or a DBus service that is needed. A simple comment would improve the packaging quality.
No. JS apps use normal shared libraries just like complied applications do, but they do it via GObject Introspection. What does it matter to other packagers who will read the spec if I need a shared library or a dbus service? (hint: it doesn't).
As pointed out before: package contents may change, files may move to other packages or subpackages, files may get renamed or deleted. That makes a dependency on a package name a weak dependency. A comment in the spec file can be helpful for other packagers, who examine inter-package dependencies with e.g. repoquery. I've opened a thread on packaging list to raise some discussion about comments on explicit Requires and suggesting that the guidelines be enhanced.
Taking for review.
If package A requires package B, thats all the info you need. Comments are needed as much as it would be if its a C app/library. I think we are just bike-shedding a non-issue now.
$ rpmlint gnome-maps-3.9.90.2-1.fc20.src.rpm \ gnome-maps-3.9.90.2-1.fc20.noarch.rpm gnome-maps.src: W: summary-not-capitalized C simple map application for GNOME desktop gnome-maps.noarch: E: explicit-lib-dependency geocode-glib gnome-maps.noarch: W: summary-not-capitalized C simple map application for GNOME desktop gnome-maps.noarch: W: no-manual-page-for-binary gnome-maps 2 packages and 0 specfiles checked; 1 errors, 3 warnings. Can you fix the summary as per Zeeshan's and Michael's suggestions and make rpmlint happy at the same time? "Map application for GNOME" sounds good to me. The rest of the rpmlint warnings are harmless and can be ignored. > +Source0: http://ftp.acc.umu.se/pub/GNOME/sources/gnome-maps/3.9/%{name}-%{version}.tar.xz Would be better to use http://download.gnome.org/sources/gnome-maps/3.9/gnome-maps-%{version}.tar.xz for the URL instead of the ftp.acc.umu.se mirror. > BuildArch: noarch It can't be noarch because /usr/bin/gnome-maps includes full paths to the GI typelibs under $libdir and these paths would differ between multilib arches. > Requires: geoclue2 > Requires: geocode-glib It would also have to depend on 'gjs' + all the packages that ship the following typelibs: $ rpm -ql gnome-maps | grep '\.js$' | xargs grep -h 'imports\.gi\.' | sort | uniq const Champlain = imports.gi.Champlain; const Clutter = imports.gi.Clutter; const Cogl = imports.gi.Cogl; const GLib = imports.gi.GLib; const GObject = imports.gi.GObject; const Gdk = imports.gi.Gdk; const GdkPixbuf = imports.gi.GdkPixbuf; const Geocode = imports.gi.GeocodeGlib; const Gio = imports.gi.Gio; const Gtk = imports.gi.Gtk; const GtkChamplain = imports.gi.GtkChamplain; const GtkClutter = imports.gi.GtkClutter; > %description > GNOME Maps is a simple map application for the GNOME desktop Sorry for nitpicking, but can you add a full stop to the end of the sentence please? > gtk-update-icon-cache %{_datadir}/icons/hicolor >&/dev/null || : > /usr/bin/glib-compile-schemas %{_datadir}/glib-2.0/schemas &> /dev/null || : Perhaps remove the path from /usr/bin/glib-compile-schemas as well so that they'd look consistent. Also, can you make it obsolete emerillon, please? It's getting retired from F20+ and would be nice to have an upgrade path. Something like this: %define emerillon_version 0.1.90-15 Obsoletes: emerillon < %{emerillon_version} Obsoletes: emerillon-devel < %{emerillon_version} Obsoletes: emerillon-vala < %{emerillon_version}
Fixed all issues. SRPM URL: http://elad.fedorapeople.org/reviews/gnome-maps/gnome-maps-3.9.90.2-2.fc20.src.rpm
Fedora review gnome-maps-3.9.90.2-2.fc20.src.rpm 2013-08-27 $ rpmlint gnome-maps-3.9.90.2-2.fc20.src.rpm \ gnome-maps-3.9.90.2-2.fc20.x86_64.rpm gnome-maps.x86_64: E: explicit-lib-dependency geocode-glib gnome-maps.x86_64: E: explicit-lib-dependency libchamplain gnome-maps.x86_64: E: explicit-lib-dependency libchamplain-gtk gnome-maps.x86_64: W: obsolete-not-provided emerillon gnome-maps.x86_64: W: obsolete-not-provided emerillon-devel gnome-maps.x86_64: W: obsolete-not-provided emerillon-vala gnome-maps.x86_64: E: no-binary gnome-maps.x86_64: W: no-manual-page-for-binary gnome-maps 2 packages and 0 specfiles checked; 4 errors, 4 warnings. + OK ! needs attention + rpmlint warnings / errors are harmless and can be ignored + The package is named according to Fedora packaging guidelines + The spec file name matches the base package name. + The package meets the Packaging Guidelines + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The license field in the spec file matches the actual license + The package contains the license file (COPYING) + Spec file is written in American English + Spec file is legible + Upstream sources match sources in the srpm. md5sum: a183744d6c78086c29ddf57816967ad2 gnome-maps-3.9.90.2.tar.xz a183744d6c78086c29ddf57816967ad2 Download/gnome-maps-3.9.90.2.tar.xz + The package builds in koji n/a ExcludeArch bugs filed + BuildRequires look sane n/a The spec file handles locales properly n/a ldconfig in %post and %postun + Package does not bundle copies of system libraries n/a Package isn't relocatable + Package owns all the directories it creates + No duplicate files in %files + Permissions are properly set + Consistent use of macros + The package must contain code or permissible content n/a Large documentation files should go in -doc subpackage + Files marked %doc should not affect package n/a Header files should be in -devel n/a Static libraries should be in -static n/a Library files that end in .so must go in a -devel package n/a -devel must require the fully versioned base + Packages should not contain libtool .la files + Proper .desktop file handling + Doesn't own files or directories already owned by other packages + Filenames are valid UTF-8 Looks good to me, just two small things with the runtime requires: 1) Missing gjs dep, can you add either 'Requires: gjs' or 'Requires: /usr/bin/gjs-console' before importing? 2) It would be safer to tighten the library requires with %{?_isa} so that it doesn't accidentally pull in the wrong multilib arch. Yum's depsolver gets easily confused. APPROVED
New Package SCM Request ======================= Package Name: gnome-maps Short Description: Map application for GNOME Owners: kalev zeenix elad Branches: f20 InitialCC:
Git done (by process-git-requests).
-debuginfo package is empty, needs to be explicitly disabled if there's nothing to extract debuginfo from.
(In reply to Ville Skyttä from comment #19) > -debuginfo package is empty, needs to be explicitly disabled if there's > nothing to extract debuginfo from. How do I do that?
https://fedoraproject.org/wiki/Packaging:Guidelines#Debuginfo_packages -> https://fedoraproject.org/wiki/Packaging:Debuginfo
Elad: ping?
Sorry for taking this long. Building a fixed package in rawhide now, will build for f20 too asap.
Oh, wow, this one is still open? I fixed it long ago. Closing.