[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-04-14 Thread bugzilla
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=696357

Mohamed El Morabity  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-04-14 Thread bugzilla
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=696357

Mohamed El Morabity  changed:

   What|Removed |Added

 CC||pikachu.2...@gmail.com
 AssignedTo|nob...@fedoraproject.org|pikachu.2...@gmail.com

--- Comment #1 from Mohamed El Morabity  2011-04-14 
12:19:15 EDT ---
I will review 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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-04-14 Thread bugzilla
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=696357

--- Comment #2 from Mohamed El Morabity  2011-04-14 
22:54:17 EDT ---
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.

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-04-14 Thread bugzilla
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=696357

--- Comment #3 from Timur Kristóf  2011-04-15 01:59:17 EDT ---
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.

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-04-26 Thread bugzilla
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=696357

--- Comment #4 from Mohamed El Morabity  2011-04-26 
20:58:38 EDT ---
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.*.

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-05-03 Thread bugzilla
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=696357

--- Comment #5 from Timur Kristóf  2011-05-03 16:59:08 EDT ---
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.

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-05-11 Thread bugzilla
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=696357

--- Comment #6 from Mohamed El Morabity  2011-05-11 
12:44:23 EDT ---
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
 1.0-2 »).
- the description is limited to 80 characters per line, not less. You have
space enough to set it in two lines ^^.

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-05-16 Thread bugzilla
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=696357

--- Comment #7 from Timur Kristóf  2011-05-16 15:47:08 EDT ---
Both of the "minor points" have been fixed.
The new files can be found at http://venemo.fedorapeople.org/sources/ (as
usual)

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-05-16 Thread bugzilla
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=696357

--- Comment #8 from Mohamed El Morabity  2011-05-17 
02:39:58 EDT ---
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/fedoral...@fedoraproject.org/ 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/fedoral...@fedoraproject.org/ 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/fedoral...@fedoraproject.org/
%doc LICENSE
%doc README

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
http

[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-09-08 Thread bugzilla
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=696357

--- Comment #9 from Mohamed El Morabity  2011-09-08 
18:59:45 EDT ---
Ping?

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-10-26 Thread bugzilla
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=696357

--- Comment #10 from Mohamed El Morabity  2011-10-26 
09:27:40 EDT ---
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

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 696357] Review Request: fedora-logo-gnome-shell-extension - adds a Fedora logo to Gnome shell's Activities button

2011-11-02 Thread bugzilla
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=696357

--- Comment #11 from Timur Kristóf  2011-11-02 07:57:30 EDT 
---
Sorry Mohamed, I haven't had the time to take care of the stuff yet. I promise
to look into it this week.

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review