[Bug 181599] Review Request: gallery: web based photo album software

Thu May 25 03:42:01 UTC 2006

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: gallery: web based photo album software

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=181599

------- Additional Comments From kevin at tummy.com  2006-05-24 23:34 EST -------
Oh, just noticed your comment about subversion. I ran into that with the
following review as well. It looks like you didn't update your spec/src.rpms
with the new BuildRequires: subversion in comment #44? (ie, the spec seems to be 
-13 still?)

See below - Rpmlint output.
See below - Package name.
OK - Spec file name matches.
OK - Package guidelines.
OK - Licsense.
OK - License field matches in spec.
OK - License included in files
OK - Spec in american english
OK - Spec legible
See below - Md5sum of source from upstream
See below - Compiles and builds on one arch at least.
OK - NO Forbidden buildrequires included
OK - All required buildrequires included
See below - Locale handling/find_lang.
OK - Owns all directories it creates.
OK - No duplicate files in %files listing.
OK - Permissions on files correct.
OK - Clean section correct.
OK - Macros consistant.
OK - Code not content.
OK - Docs must not affect runtime.
OK - Doesn't own any files/dirs that are already owned by others.

See below - Package builds in mock.
See below - Subpackages require base package.

1. Is there any further need to base the package off a snapshot instead of
the now current stable release?

2. If the answer to 1 is yes, should the package have svn in the release
instead of cvs?
See:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-d97a3f40b6dd9d2288206ac9bd8f1bf9b791b22a

3. The Requires for the subpackages should probibly be:
Requires: %{name} = %{version}-%{release}
Otherwise you can get diffrent 2.1 versions of the various packages installed
at the same time.

4. Doesn't build in mock here... I get the following at the end of
the build.log:

+ perl makeManifest.pl
Finding all files...Died at makeManifest.pl line 153.
error: Bad exit status from /var/tmp/rpm-tmp.63104 (%build)

which is a call to svn:
  open(FD, "svn propget --non-interactive -R svn:mime-type |") or die;

Perhaps a missing BuildRequires: subversion
I can't tell for sure if this will hit the network during a build.
If so, it will fail in the buildsystem. Basing on a non snapshot might
fix this issue as well.

5. Can you provide a script used to generate the svn tar.gz snapshot
or otherwise describe how to duplicate it? Switching back to the stable
version would remove this requirement as well. ;)

6. Perhaps a README.fedora that has info on how to setup
the database easily? Or pointers to the docs to do that?

7. You need to use the find_lang macro/setup for locales, you can't
just include them in files sections, See:

http://fedoraproject.org/wiki/Packaging/Guidelines?#head-8c605ebf8330f6d505f384e671986fa99a8f72ee

8. Lots of rpmlint output:

W: gallery2 incoherent-version-in-changelog 2.0-0.13.cvs20060303
2.1-0.14cvs20060524.fc6
E: gallery2 htaccess-file /var/www/gallery2/lib/tools/po/.htaccess
E: gallery2 htaccess-file /var/www/gallery2/.htaccess
E: gallery2 htaccess-file /var/www/gallery2/lib/tools/bin/.htaccess
E: gallery2 htaccess-file /var/www/gallery2/lib/tools/stubs/.htaccess
E: gallery2-rewrite htaccess-file
/var/www/gallery2/modules/rewrite/data/mod_rewrite_no_options/gallery/.htaccess
E: gallery2-rewrite htaccess-file
/var/www/gallery2/modules/rewrite/data/mod_rewrite/gallery/.htaccess
E: gallery2-rewrite htaccess-file
/var/www/gallery2/modules/rewrite/data/mod_rewrite_no_options/custom/.htaccess
E: gallery2-rewrite htaccess-file
/var/www/gallery2/modules/rewrite/data/mod_rewrite_no_options/gallery2/.htaccess
E: gallery2-rewrite htaccess-file
/var/www/gallery2/modules/rewrite/data/mod_rewrite/gallery2/.htaccess
E: gallery2-rewrite htaccess-file
/var/www/gallery2/modules/rewrite/data/mod_rewrite/custom/.htaccess

Suggest from rpmlint:

htaccess-file :
You have individual apache configuration .htaccess file(s) in your package.
Replace them by a central configuration file in /etc/httpd/conf.d.
Is it possible to fold them into the main gallery2.conf?

E: gallery2 non-executable-script
/var/www/gallery2/lib/tools/po/premerge-messages.pl 0644
E: gallery2 non-executable-script
/var/www/gallery2/lib/tools/po/update-all-translations.pl 0644
E: gallery2 non-executable-script
/var/www/gallery2/lib/tools/bin/getIllegalFunctions.pl 0644
E: gallery2 non-executable-script
/var/www/gallery2/lib/tools/uml/make-java-classes.pl 0644
E: gallery2 non-executable-script /var/www/gallery2/lib/tools/po/header.pl 0644

Should those scripts be 755?

E: gallery2 non-standard-uid /srv/gallery2 apache
E: gallery2 non-standard-uid /var/www/gallery2/config.php apache

I think those can be ignored.

E: gallery2 no-jar-manifest
/var/www/gallery2/modules/core/classes/GalleryStorage/g2_db2.jar

Something to complain to upstream about?

There are tons of these... basically for each subpackage:

W: gallery2-albumselect summary-not-capitalized albumselect module for Gallery 2
W: gallery2-albumselect no-documentation

No documentation can be ignored.
You should go ahead and capitalize the first letter of each Summary.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.