Spec URL: http://sources.venemo.net/fedora-logo-gnome-shell-extension.spec SRPM URL: http://sources.venemo.net/fedora-logo-gnome-shell-extension-1.0.0-1.fc15.src.rpm Description: fedora-logo-gnome-shell-extension is a simple Gnome shell extension that adds the Fedora logo to the Activities button of the Gnome shell. This is a gnome-shell extension that adds a Fedora logo to the Activities button of the Shell. (On right-to-left locales it appears on the top right corner, otherwise on the top left corner.) The extension is supposed to work with any Gnome shell theme. (Tested with some popular ones and the default one.) Link to the idea on the design team's mailing list: http://lists.fedoraproject.org/pipermail/design-team/2011-April/004215.html
I will review this package.
A quite good package, simple but works well :). A few comments anyway: * maybe your package should be renamed « gnome-shell-extension-fedora-logo » or « gnome-shell-fedora-logo », at least to respect the naming guidelines for addons packages (see http://fedoraproject.org/wiki/PackageNamingGuidelines#Addon_Packages_.28General.29). * the fedora-logos package provides a « system-logos » capacity, as well as the generic-logos package (and probably the redhat-logos in RHEL also). Why not setting system-logos as Requires instead of fedora-logos, so that your package would be usable without any change in Fedora as well as in any Fedora-derivated distribution? * about the URL tag: why not simply use http://sources.venemo.net/? The URL tag is intended to point to the project website, no matter how small it is. If it's not the solution you prefer, you could create a basic page in your fedorapeople.org space (or wherever you can) containing the description of the package and links to the sources.
Hello Mohamed, thank you for reviewing! :) (In reply to comment #2) > * maybe your package should be renamed « gnome-shell-extension-fedora-logo » or > « gnome-shell-fedora-logo Because if I named it like that, people would think that the package is provided by upstream (like gnome-shell-extensions), which is not the case. And if you look at any of such Fedora packages, all of them begin with either the name "fedora" or a release name. (examples: fedora-icon-theme, lovelock-backgrounds, laughlin-kde-theme, etc.) If you still think that the name needs to change, I will rename it. > * the fedora-logos package provides a « system-logos » capacity, as well as the > generic-logos package (and probably the redhat-logos in RHEL also). Why not > setting system-logos as Requires instead of fedora-logos, so that your package > would be usable without any change in Fedora as well as in any Fedora-derivated > distribution? Very good point, I wasn't aware of such a possibility. Currently the extension finds the logo by the icon name 'fedora-logo-icon'. If you tell me what icon name to use in order to utilize this system-logos capacity, I will gladly change it. :) > * about the URL tag: why not simply use http://sources.venemo.net/? The URL tag > is intended to point to the project website, no matter how small it is. If it's > not the solution you prefer, you could create a basic page in your > fedorapeople.org space (or wherever you can) containing the description of the > package and links to the sources. I think making a fedorapeople.org page is a good idea, I'll make one.
Really really sorry for this late answer. I may reconsider my views about system-logos, since there is in fact no generic equivalent of the fedora-logo file in the generic-logos package. So I tried to find an alternative. The proper way to get the logo whatever the distribution may be to refer to start-here; this is by the way the icon grabbed by gnome-panel in GNOME 2. The fedora-logos package contains start-here.* icon files, as symbolink links to fedora-icon.*, in /usr/share/icons/hicolor/, since any theme inherits from the hicolor theme. In Fedora<=14, the default theme used in GNOME was the "fedora" one, based on mist which doesn't provide any start-here icon, so start-here was grabbed in hicolor. generic-logos doesn't provide any start-here.* icon, so the "fallback" GNOME start-here.* icon was used instead (the foot logo). In Fedora 15, it seems the default icon theme is "gnome", which provides its own start-here icon, that is to say the foot logo. Some heuristics to grab a path to the distribution logo whatever the current icon theme may exist. It may be an evolution for this extension, one day ^^. Anyway I'm OK to approve this package as it is currently if you rename it to gnome-shell-extension-fedora-logo, which seems to me finally the most appropriate name according to the guidelines. Be *very* careful also with the GNOME Shell version set in metadata.json: you set the shell-version key to 3.0.0.2 whereas the latest version available of GS in the repos is 3.0.1. As a result, your extension won't work in GS > 3.0.0.2. A workaround would be to set shell-version to 3.0, which seems to work for any GS version 3.0.*.
Hello Mohamed, I've made the modifications you requested. I renamed the package and added an "Obsoletes: fedora-logo-gnome-shell-extension" line to the .spec so that the Shell doesn't crash for those people who have the package with the original name. I also took your advice and put it onto fedorapeople. The new .spec, .src.rpm, and tarball are here: http://venemo.fedorapeople.org/sources/ The SCM is here: git://fedorapeople.org/~venemo/gnome-shell-extension-fedora-logo.git I hope it is okay now.
Sorry (again!) for this late answer. The package is indeed OK. Just two minor points to fix: - set the version-release in the first entry of the changelog according to the ones defined in the .spec (fixed string : « * Sun Apr 10 2011 Timur Kristóf <venemo> 1.0-2 »). - the description is limited to 80 characters per line, not less. You have space enough to set it in two lines ^^.
Both of the "minor points" have been fixed. The new files can be found at http://venemo.fedorapeople.org/sources/ (as usual)
Here is the review. MUST: rpmlint must be run on every package. ->OK MUST: The package must be named according to the Package Naming Guidelines. ->OK MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. ->OK MUST: The package must meet the Packaging Guidelines. ->OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. ->OK MUST: The License field in the package spec file must match the actual license. ->OK MUST: 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 must be included in %doc. ->OK MUST: The spec file must be written in American English. ->OK MUST: The spec file for the package MUST be legible. ->OK MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. ->OK MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. ->OK MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. ->N/A MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. ->OK MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden. ->N/A MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. ->N/A MUST: Packages must NOT bundle copies of system libraries. ->OK MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. ->N/A MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. ->NOK => /usr/share/gnome-shell/extensions/fedoralogo/ is not owned by the package MUST: A Fedora package must not list a file more than once in the spec file's %files listings. ->OK MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. ->OK MUST: Each package must consistently use macros. ->OK MUST: The package must contain code, or permissable content. ->OK MUST: Large documentation files must go in a -doc subpackage. ->N/A MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. ->OK MUST: Header files must be in a -devel package. ->N/A MUST: Static libraries must be in a -static package. ->N/A MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. ->N/A MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}. ->N/A MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. ->N/A MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. ->N/A MUST: Packages must not own files or directories already owned by other packages. ->OK MUST: All filenames in rpm packages must be valid UTF-8. ->OK Just one issue, mentionned above:%{_datadir}/gnome-shell/extensions/fedoralogo/ is created by the packaged but not owned. To fix it, you can set your %files section as below: %files %defattr(-,root,root,-) %{_datadir}/gnome-shell/extensions/fedoralogo/ %doc LICENSE %doc README
Ping?
There is unfortunately no comment on this review for more than a month. Without any action within a week, the review will be closed. http://fedoraproject.org/wiki/Package_maintainer_policy#Reviewer_not_responding
Sorry Mohamed, I haven't had the time to take care of the stuff yet. I promise to look into it this week.
Ping? Any progress here? Or we can close this review?
I think there are other extensions by now which can fulfill the same purpose. So I'm closing this as WONTFIX.