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=481272 Tadej Janež <tadej.janez@xxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tadej.janez@xxxxxxxxxxxxxxx | |si --- Comment #2 from Tadej Janež <tadej.janez@xxxxxxxxxxxxxxxxx> 2009-01-25 12:03:50 EDT --- This is my informal review. I cannot sponsor you as I'm not (yet) an approved packager. However, my review can help your future sponsor when he makes his own official review. Good: + rpmlint output is clean. + The package is named according to the Package Naming Guidelines. + The spec file name matches the base package %{name}, in the format %{name}.spec. + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The License field in the package spec file matches the actual license. + The spec file is written in American English. + The spec file for the package is legible. + The sources used to build the package match the upstream source, as provided in the spec URL (md5sum webunit-1.3.8.tar.gz: 97b9e6b5149dadce48b86adbf2db3b0a). + The package successfully compiles and builds into binary rpms on at least one primary architecture. + All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines. + The package owns all directories that it creates. + The package doesn't contain any duplicate files in the %files listing. + Permissions on files are set properly. + The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). + The package consistently uses macros. + The package contains code, or permissible content. + Package doesn't own files or directories already owned by other packages. + At the beginning of %install, package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT). + All filenames in rpm packages are valid UTF-8. + I tested that the package builds in mock. + The package compiles and builds into binary rpms on all supported architectures. + I did a limited test that the package functions as described. + egg-info files which are generated by the module's build scripts are included in the package. Comments: - You should use %{python_sitelib} instead of %{python_sitearch}, because your package is architecture independent (noarch) - You should remove %{python_sitearch} macro definition as you don't need it. - You should remove CFLAGS=... in %build section because this a noarch package. - You should avoid putting demo folder in the top-level %{python_sitelib} folder and rather put in a subfolder like %{python_sitelib}/%{Project_name}. - You could shorten the %files section by using: %{python_sitearch}/%{Project_name}/ instead of: %{python_sitearch}/%{Project_name}/HTMLParser.py* %{python_sitearch}/%{Project_name}/IMGSucker.py* %{python_sitearch}/%{Project_name}/SimpleDOM.py* %{python_sitearch}/%{Project_name}/__init__.py* %{python_sitearch}/%{Project_name}/config.py* %{python_sitearch}/%{Project_name}/cookie.py* %{python_sitearch}/%{Project_name}/utility.py* %{python_sitearch}/%{Project_name}/webunittest.py* And similarly for the demo directory. - The version in the %changelog section should be on the same line as the date and your name. - Because the source package does not include license text(s) as a separate file from upstream, you should query upstream to include it. - You should improve the Summary and Description fields. Look at other packages for examples. Your summary should state something like "A python module for unit testing websites acting like a web browser". - Optionally, you could shorten and rename %{Project_name} macro to something like %{project} or %{module}. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review