Bug 521569 - Review Request: perl-Locale-Maketext-Gettext - Joins the gettext and Maketext frameworks
Review Request: perl-Locale-Maketext-Gettext - Joins the gettext and Maketext...
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 528347
  Show dependency treegraph
 
Reported: 2009-09-06 19:13 EDT by Ruediger Landmann
Modified: 2013-06-07 08:47 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-11-03 19:34:50 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
tibbs: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ruediger Landmann 2009-09-06 19:13:35 EDT
Spec URL: http://rlandmann.fedorapeople.org/perl-Locale-Maketext-Gettext.spec
SRPM URL: http://rlandmann.fedorapeople.org/perl-Locale-Maketext-Gettext-1.27-1.fc11.src.rpm
Description: Locale::Maketext::Gettext joins the GNU gettext and Maketext frameworks. It is a subclass of Locale::Maketext(3) that follows the way GNU gettext works. You start as a usual GNU gettext localization project: Work on PO files with the help of translators, reviewers and Emacs. Turn them into MO files with msgfmt. Copy them into the appropriate locale directory, such as /usr/share/locale/de/LC_MESSAGES/myapp.mo. Then, build your Maketext localization class, with your base class changed from Locale::Maketext(3) to Locale::Maketext::Gettext.
Comment 1 Ruediger Landmann 2009-09-06 19:24:20 EDT
This is my first package, so I'm seeking a sponsor please.
Comment 2 Jason Tibbitts 2009-09-18 15:04:07 EDT
Builds fine and rpmlint is silent.  The spec, being cpanspec generated, is nice and clean.

I note that 1.28 is out, released well before you posted this review request.  I checked the upstream changelog and it doesn't seem that there are any functional changes, so I'll go ahead and review the current version.  However, beware that if you do update, there may be some dependency issues because packages reqired for the test suite are no longer listed in Build.PL.

I note that the test suite skips one test:
  t/99-pod.t ........... skipped: Test::Pod 1.00 required for testing POD
Generally this is a maintainer test anyway and its value to us is somewhat minimal, but I would suggest that you add a build dependency on perl(Test::Pod) anyway, because you'll at least be alerted of any issues with the documentation.  The package still builds fine if you do this.

If you fix that one minor issue, I'll approve this package and sponsor you.

* source files match upstream.  sha256sum:
   3834e317e49e7bee4133f82d41cbc43e8eda92c6cdcde6c282021c083e0dc8f0  
   Locale-Maketext-Gettext-1.27.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
   perl(Locale::Maketext::Gettext) = 1.27
   perl(Locale::Maketext::Gettext::Functions) = 0.13
   perl(Locale::Maketext::Gettext::Functions::_EMPTY) = 0.01
   perl(Locale::Maketext::Gettext::Functions::_EMPTY::i_default) = 0.01
   perl-Locale-Maketext-Gettext = 1.27-1.fc12
  =
   /usr/bin/perl
   perl >= 0:5.008
   perl(:MODULE_COMPAT_5.10.0)
   perl(Encode)
   perl(File::Spec::Functions)
   perl(Getopt::Long)
   perl(Locale::Maketext::Gettext)
   perl(Locale::Maketext::Gettext::Functions)
   perl(base)
   perl(strict)
   perl(vars)
   perl(warnings)

? %check is present and all tests pass:
   All tests successful.
   Files=13, Tests=350,  2 wallclock secs ( 0.10 usr  0.02 sys +  0.93 cusr  
    0.26 csys =  1.31 CPU)
  However, one test is skipped due to a missing build dependency.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
Comment 3 Ruediger Landmann 2009-09-20 19:28:15 EDT
Thanks for the catch, Jason.

I've updated the Specfile and .src.rpm to include the extra build dependency that you recommended, and the package now passes t/99-pod.t as well:

https://koji.fedoraproject.org/koji/getfile?taskID=1693616&name=build.log
Comment 4 Jason Tibbitts 2009-09-21 19:41:01 EDT
Sorry,  could you provide links to the updated spec and src.rpm?  All I see is the original 1.27-1.  (If that's not the same 1.27-1 that I originally reviewed, now I suppose you know why we request that you increase Release: when you modify your submission.)
Comment 5 Ruediger Landmann 2009-09-21 22:41:31 EDT
Sorry -- I didn't realise that I needed to start increasing the Release number before the package was in Fedora. Thanks for the pointer!

The new .src.rpm is here:
http://rlandmann.fedorapeople.org/perl-Locale-Maketext-Gettext-1.27-2.fc11.src.rpm

and the new spec file is here:
http://rlandmann.fedorapeople.org/perl-Locale-Maketext-Gettext.spec

Scratch build in Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1696557
Comment 6 Jason Tibbitts 2009-09-22 19:17:27 EDT
Looks good to me, thanks.

APPROVED

I've clicked the necessary button in FAS, so you should be able to make your CVS request as soon as that propagates through the system.  That generally happens within an hour.   Please let me know if you need further assistance getting your package built and pushed out.  It's simplest to contact me on IRC in #fedora-devel (tibbs or tibbs|h) but you are also welcome to email me.

Also, I note that there are a couple of other perl packages submitted bu Red Hat folks; I checked and it seems as if all three of you are in the docs group.  If you know what's going on, or you'd like me to help with that, please let me know.
Comment 7 Ruediger Landmann 2009-09-23 14:51:06 EDT
Thanks very much.

I'm having a little trouble at the moment, as my address in Bugzilla is different from my FAS address, but I'll get that sorted out. If I have any further drama after that, I'll be sure to take you up on your offer for extra help! :)

We're trying (again) to get some of the docs group involved with packaging some of the modules that are relevant to the tools we use -- so these are the other perl packages that you've noticed. We're all (obviously) new at this, so if you wouldn't mind taking a look at Bug 521723 and Bug 521724 as well and let Scott and Ryan know if they're on the right track, we'd all be really grateful :)

Cheers
Rudi
Comment 8 Ruediger Landmann 2009-09-24 23:01:33 EDT
New Package CVS Request
=======================
Package Name: perl-Locale-Maketext-Gettext
Short Description: Joins the gettext and Maketext frameworks
Owners: rlandmann
Branches: F-10 F-11 F-12 EL-4 EL-5
InitialCC:
Comment 9 Ralf Corsepius 2009-09-24 23:18:33 EDT
Please add perl-sig to InitialCC.
Comment 10 Ruediger Landmann 2009-09-24 23:24:17 EDT
New Package CVS Request
=======================
Package Name: perl-Locale-Maketext-Gettext
Short Description: Joins the gettext and Maketext frameworks
Owners: rlandmann
Branches: F-10 F-11 F-12 EL-4 EL-5
InitialCC:  perl-sig
Comment 11 Kevin Fenzi 2009-09-25 12:34:51 EDT
cvs done.
Comment 12 Ruediger Landmann 2013-06-07 02:38:29 EDT
Package Change Request
======================
Package Name: perl-Locale-Maketext-Gettext
New Branches: el6
Owners: rlandmann
Comment 13 Jon Ciesla 2013-06-07 08:47:05 EDT
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.