Bug 228969

== Not an official review as I'm not yet sponsored ==

The blockers are listed below

      - MUST: The spec file name must match the base package %{name}, in the
format %{name}.spec unless your package has an exemption on Package Naming
Guidelines.

spec name should conform to package name: wxGlade

      - MUST: The package must meet the Packaging Guidelines.

The website says:
Starting from version 0.4, wxGlade requires wxPython >= 2.6 and Python >= 2.2.

should not Requires(pre) or Requires(post) desktop-file-utils, but should
BuildRequires it.

BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

%description has a blank comment?

install icons to hicolors directory instead of pixmaps, and adjust your desktop
file (Icon=wxGlade without file extension). This will make it better suit
desktop theme changes. Remember to update icon cache if you do this.

change your desktop file to remove "Application" category, leaving only
"Development"

      - MUST: The spec file for the package MUST be legible. If the reviewer is
unable to read the spec file, it will be impossible to perform a review. Fedora
is not the place for entries into the Obfuscated Code Contest
(http://www.ioccc.org/).

better remove the sed script, to source/patch file. Is there any guidelines for
desktop files versioning? I mean, both the file name and the file content should
contain a version number, then there would be a lot of maintaince work. But if
using sed, the code will be less clear.

      - MUST: The package must successfully compile and build into binary rpms
on at least one supported architecture.

builds fine here on i386

      - MUST: All build dependencies must be listed in BuildRequires, except for
any that are listed in the exceptions section of Packaging Guidelines; inclusion
of those as BuildRequires is optional. Apply common sense.

BuildRequires desktop-file-utils

      - 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.

please do this.

      - 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.

%defattr(-,root,root,-)

      - MUST: Each package must consistently use macros, as described in the
macros section of Packaging Guidelines.

I don't know if re-define %name and %version and %release is a good idea.


Why python source and compiled files are placed in /usr/share/ directory? I
think they should be in /usr/lib/python2.x/site-packages.

I have not install this package yet.

(In reply to comment #1)
> == Not an official review as I'm not yet sponsored ==

Well, then don't assign this bug to yourself for now...
Still you can do a pre-review for this review request.

updated :
spec : ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxglade.spec
SRPM : ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxglade-0.4.1-2.fc6.src.rpm

* Sun Feb 25 2007 ZC Miao <hellwolf.misty> - 0.4.1-2
- install icon to hicolor directory
- change name to wxglade
- BuildRequires desktop-file-utils
- remove Application category in desktop file
- remove some macro redefination

* Fri Feb 16 2007 ZC Miao <hellwolf.misty> - 0.4.1-2
- Add missing icons

(In reply to comment #1)
>
> 
> better remove the sed script, to source/patch file. Is there any guidelines for
> desktop files versioning? I mean, both the file name and the file content should
> contain a version number, then there would be a lot of maintaince work. But if
> using sed, the code will be less clear.

desktop files don't need to have a version.


> I don't know if re-define %name and %version and %release is a good idea.

It is definitely a bad idea.
 
> Why python source and compiled files are placed in /usr/share/ directory? I
> think they should be in /usr/lib/python2.x/site-packages.

Only modules to be used by other python applications. 
If the python files are only used internally by 
the application the best place is along %{_datadir}/%{name}.

updated:
SPEC : ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxglade.spec
SRPM : ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxglade-0.4.1-3.fc6.src.rpm

%changelog
* Tue Feb 27 2007 ZC Miao <hellwolf.misty> - 0.4.1-3
- Desktop entry do not need version number
- remove some comments

(In reply to comment #5)
> updated:
> SPEC : ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxglade.spec
> SRPM : ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxglade-0.4.1-3.fc6.src.rpm
> 

== formal review ==

The latest version is wxGlade-0.5.

The package name, as well as spec file name should be "wxGlade" instead of
"wxglade". Or, have you contacted the authors?

In the launcher script, \$@ should be quoted with double quote. 

Mixed tabs and spaces in spec file.

Why using "Prefix:".

desktop-file-install --vendor fedora instead of ""


Thanks.

Updated

ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxGlade.spec
ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxGlade-0.5-1.fc6.src.rpm

"\$@" is not fixed.

in %prep, %setup should not use -n param

in %install, the "install" command should taken care about directory permission
and preserve file time stamp.

Thanks.

Updated :

ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxGlade-0.5-2.fc6.src.rpm
ftp://ftp.fedora.cn/pub/fedora-cn/in-review/wxGlade.spec

Approved. 

Well, the fedora-review flag has long been set to '+'. Who did that?

Please request to add a CVS module on your own. Thanks.

Comment:

You did, https://bugzilla.redhat.com/bugzilla/show_activity.cgi?id=228969

http://fedoraproject.org/wiki/ seems down currently.
And I forget(but never remembered) the next step :(

New Package CVS Request
=======================
Package Name: wxGlade
Short Description: A wxWidgets/wxPython/wxPerl GUI designer
Owners: hellwolf.misty
Branches: FC-6
InitialCC: bbbush.yuan

(In reply to comment #11)
> Comment:
> 
> You did, https://bugzilla.redhat.com/bugzilla/show_activity.cgi?id=228969

Thanks. Maybe a mistake.

Confirmed, approved.

Hellwolf, please continue to set fedora-cvs flag and request for cvs modules.
Thanks.