Bug 591389

I'd like to review this today, but the srpm is not available as the server is down. I already contacted Jeroen.

Some comments on the spec:

License tag is invalid, the version of the GPL needs to be specified.

Please use the full length of 80 characters for the description.

Please use -p when using install to preserve timestamps.

localized manpages need to be tagged as such with the %lang macro. Thus you should use '%find_lang %{name} --with-man' and not do this in the %files section.

README is pretty useless.

The changelog mentions a BR for debhelper, but it's not in the build requirements (and as not part of dpkg-devel ether).

Ping. Please provide public URLs for the SRPM and the spec.

New SPEC: http://git.kolabsys.com/rpm/po-debconf/plain/po-debconf.spec?h=f13/master
New SRPM: http://mirror.kolabsys.com/pub/fedora/apt-utils/f13/SRPMS/po-debconf-1.0.16-3.fc13.src.rpm

New SPEC (Release: 4%dist):
  http://oron.fedorapeople.org/deb-package/po-debconf.spec
New SRPM:
  http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16-4.fc13.src.rpm

Fixes and cleanups:
1. Non-specific License: GPL -> GPLv2
2. Replaced po-debconf-1.0.16-fix-prefix.patch with a
   bigger po-debconf-1.0.16-make-install.patch:
     * prefix appeared more than once (although the second time looked
       unused)
     * Added 'install' rule:
       - Refactored from debian/rules and our .spec file
       - Use the common DESTDIR=
       - Define and use common installation directories (prefix, bindir,
         datadir, etc.)
       - This should be pushed to upstream (Debian) so logic isn't duplicated.
         If they apply this, they can (optionally) streamline their debian/rules
         [and then it may become good candidate for dh(1)]
3. Man pages:
     * Installation: was left outside of Makefile 'install' since debian
       (rightfully) use their dh_installman (no common code with Fedora)
     * Installation from .spec file was fixed:
       - Strip the language code from the file basename (foo.1 not foo.de.1)
       - Generalize so all man sections are installed, not only section 1

TODO:
   - Apply %find_lang logic?
   - Send Makefile patch upstream
   - Anything else?

Results of rpmlint for package in comment 5:
po-debconf.src: W: spelling-error Summary(en_US) gettext -> get text, get-text, getter
po-debconf.src: W: spelling-error %description -l en_US utils -> utile, utilizes, utilize
po-debconf.src: W: spelling-error %description -l en_US gettext -> get text, get-text, getter
po-debconf.noarch: W: spelling-error %description -l en_US utils -> utile, utilizes, utilize
2 packages and 1 specfiles checked; 0 errors, 4 warnings.

I am a little confused. Who is submitting this package and who is reviewing it? It's nice that Oron helps out, but IMHO the reviewer should not work on the package (and contrariwise).


(In reply to comment #5)
> New SPEC (Release: 4%dist):
>   http://oron.fedorapeople.org/deb-package/po-debconf.spec
> New SRPM:
>   http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16-4.fc13.src.rpm
> 
> Fixes and cleanups:
> 1. Non-specific License: GPL -> GPLv2

If no version is specified, this usually means that "any later version" is included, see paragraph 9 of the GPLv2.

>      * Installation: was left outside of Makefile 'install' since debian
>        (rightfully) use their dh_installman (no common code with Fedora)
>      * Installation from .spec file was fixed:
>        - Strip the language code from the file basename (foo.1 not foo.de.1)
>        - Generalize so all man sections are installed, not only section 1

Well done!

> TODO:
>    - Apply %find_lang logic?

Yes please

>    - Send Makefile patch upstream

+1

>    - Anything else?

I'd like to see the HTML docs included, but the rest looks good to me from a quick glance.

(In reply to comment #6)
> Results of rpmlint for package in comment 5:
> po-debconf.src: W: spelling-error Summary(en_US) gettext -> get text, get-text,
> getter
> po-debconf.src: W: spelling-error %description -l en_US utils -> utile,
> utilizes, utilize
> po-debconf.src: W: spelling-error %description -l en_US gettext -> get text,
> get-text, getter
> po-debconf.noarch: W: spelling-error %description -l en_US utils -> utile,
> utilizes, utilize
> 2 packages and 1 specfiles checked; 0 errors, 4 warnings.

These can be ignored.

> I am a little confused. Who is submitting this package and who is reviewing it?
> It's nice that Oron helps out, but IMHO the reviewer should not work on the
> package (and contrariwise).

I'm not the reviewer, so no problem here. As someone who want to have Debian
packaging toolchain on my Fedora box, I have no problem either just helping out,
co-maintaining, or maintaining this package collection (po-debconf, dh-make,
debhelper, dpkg, pbuilder, etc.) as kanarip/yourself would like -- whatever
suites best.

New SPEC:
    http://oron.fedorapeople.org/deb-package/po-debconf.spec
New SRPM:
    http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16-5.fc13.src.rpm

1. License: GPLv2 -> GPLv2+
2. Added %find_lang logic
3. Makefile patch sent to Debain BTS (Though I don't see it yet...)
       http://bugs.debian.org/cgi-bin/pkgreport.cgi?pkg=po-debconf
4. HTML docs add. Although they only include 'vi' translation right now
   (not even English).

Reference: link to Debian bug report:
    http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=601941

"dino" is my local repo.

# yum install po-debconf --enablerepo=dino
Setting up Install Process
Resolving Dependencies
--> Running transaction check
---> Package po-debconf.noarch 0:1.0.16-5.fc14 set to be installed
--> Processing Dependency: perl(Debconf::Template::Transient) for package: po-debconf-1.0.16-5.fc14.noarch
--> Processing Dependency: perl(Debconf::Db) for package: po-debconf-1.0.16-5.fc14.noarch
--> Processing Dependency: perl(Debconf::AutoSelect) for package: po-debconf-1.0.16-5.fc14.noarch
--> Processing Dependency: perl(Debconf::Config) for package: po-debconf-1.0.16-5.fc14.noarch
--> Finished Dependency Resolution
Error: Package: po-debconf-1.0.16-5.fc14.noarch (dino)
           Requires: perl(Debconf::Db)
Error: Package: po-debconf-1.0.16-5.fc14.noarch (dino)
           Requires: perl(Debconf::Template::Transient)
Error: Package: po-debconf-1.0.16-5.fc14.noarch (dino)
           Requires: perl(Debconf::Config)
Error: Package: po-debconf-1.0.16-5.fc14.noarch (dino)
           Requires: perl(Debconf::AutoSelect)

Those modules are from debconf project. So, it needs to add Requires: debconf. Sure, debconf is not in Fedora repo *smile*. But in Depends I put its ReviewRequest id.

Problem corrected by a newer version of the debconf package.

1. debconf is now in Fedora (#591332 -- will be closed automatically in few days
   when packages get enough karma) -- this will unblock this Review-Request.

2. Updates:
   SPEC: http://oron.fedorapeople.org/deb-package/po-debconf.spec
   SRPM: http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16+nmu1-1.fc16.src.rpm

   Changes:
   - Installed translated man pages to correct names (without $LANG in the
     man-page name, only in the prefixing directory)
   - Use find_lang for translated man-pages
   - Don't specify exact compression scheme for (non-tranlated) man-pages
   - Removed Build-Root (not needed for modern Fedora)
   - Removed buildroot removal in install step (not needed for modern Fedora)
   - Fix minor upstream permission bug (COPYING was executable)

3. rpmlint (*.rpm):
   po-debconf.noarch: W: spelling-error %description -l en_US utils -> utilizes
   po-debconf.noarch: E: script-without-shebang /usr/share/po-debconf/pot-header

Note: the "pot-header" is NOT a script. Just as its name implies it's a header
      for pot files and contains '#' comments which misslead rpmlint.

   po-debconf.src: W: spelling-error Summary(en_US) gettext -> get text, get-text, Georgette
   po-debconf.src: W: spelling-error %description -l en_US utils -> utilizes
   po-debconf.src: W: spelling-error %description -l en_US gettext -> get text, get-text, Georgette
2 packages and 0 specfiles checked; 1 errors, 4 warnings.

 * Formal reviewer (cwickert) hasn't responded since 2010-10-30.

 * This RR blocks debhelper which is needed to fix bug #716298
   (Cannot install dh-make - missing dependency)

 * debconf RR (bug #591332) which blocked this is already pushed by bodhi for F16:
     https://admin.fedoraproject.org/updates/FEDORA-2012-7824/debconf-1.5.42-5.fc16

Sorry it took so long.


Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines. (GPLv2+)
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[-]: MUST %build honors applicable compiler flags or justifies otherwise (noarch)
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[!]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean is needed only if supporting EPEL
[x]: MUST Sources contain only permissible code or content.
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files -f po-debconf.lang section. This is
     OK if packaging for EPEL5. Otherwise not needed
[-]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: 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 is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
     (GPLv2+)
[x]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[!]: MUST Requires correct, justified where necessary.
[!]: MUST Rpmlint output is silent.

rpmlint po-debconf-1.0.16+nmu1-1.fc18.src.rpm

po-debconf.src: W: spelling-error Summary(en_US) gettext -> get text, get-text, Georgette
po-debconf.src: W: spelling-error %description -l en_US utils -> tills
po-debconf.src: W: spelling-error %description -l en_US gettext -> get text, get-text, Georgette
1 packages and 0 specfiles checked; 0 errors, 3 warnings.


rpmlint po-debconf-1.0.16+nmu1-1.fc18.noarch.rpm

po-debconf.noarch: W: spelling-error %description -l en_US utils -> tills
po-debconf.noarch: E: script-without-shebang /usr/share/po-debconf/pot-header
1 packages and 0 specfiles checked; 1 errors, 1 warnings.


[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/chris/591389/po-debconf_1.0.16+nmu1.tar.gz :
  MD5SUM this package     : 959fdc68f532449df481dd09b044d739
  MD5SUM upstream package : 959fdc68f532449df481dd09b044d739

[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[-]: MUST Useful -debuginfo package or justification otherwise (noarch).
[x]: SHOULD Reviewer should test that the package builds in mock.
[-]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: SHOULD Package functions as described.
[!]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[!]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Patch0: po-debconf-1.0.16-fix-prefix.patch (po-debconf-1.0.16-fix-
     prefix.patch) Patch1: po-debconf-1.0.16-no-utf8-to-pod2man.patch (po-
     debconf-1.0.16-no-utf8-to-pod2man.patch)
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures (noarch package).
[!]: SHOULD %check is present and all tests pass.
[!]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean is needed only if supporting EPEL
See: http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files -f po-debconf.lang section. This is
     OK if packaging for EPEL5. Otherwise not needed
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[!]: MUST Rpmlint output is silent.

rpmlint po-debconf-1.0.16+nmu1-1.fc18.src.rpm

po-debconf.src: W: spelling-error Summary(en_US) gettext -> get text, get-text, Georgette
po-debconf.src: W: spelling-error %description -l en_US utils -> tills
po-debconf.src: W: spelling-error %description -l en_US gettext -> get text, get-text, Georgette
1 packages and 0 specfiles checked; 0 errors, 3 warnings.


rpmlint po-debconf-1.0.16+nmu1-1.fc18.noarch.rpm

po-debconf.noarch: W: spelling-error %description -l en_US utils -> tills
po-debconf.noarch: E: script-without-shebang /usr/share/po-debconf/pot-header
1 packages and 0 specfiles checked; 1 errors, 1 warnings.


See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint


Generated by fedora-review 0.1.3
External plugins:


Real issues: 
Most of the things fedora-review complains about are false positives, e.g. the spelling errors from rpmlint. The real issues are:

- Naming is not correct: the non-numeric part of the version needs to co into the release, see https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-Numeric_Version_in_Release

- Although the patches are trivial, you should add some notes about them to the spec: What do they do, what is the status of upstreaming them? Are they downstream only? For more info, see http://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

- please preserve timestamps during in %install, see http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

- missing dependencies: podebconf-report-po requires perl(Compress::Zlib) and perl(Mail::Sendmail) for compressing and mailing files. rpmbuild doesn't catch them because they are conditionals, but IHMO we should add dependencies.
If these modules are not installed, the script throws errors:
 "This program requires the libmail-sendmail-perl package" and 
 "This program requires the libcompress-zlib-perl package ..."
These are Debian package names, in Fedora they are called perl-Mail-Sendmail and perl-IO-Compress. You could fix that with a patch, but if we add a dependency, that should not be necessary.

- trivial: buildroot vs. "Build-Root" in %changelog

- please add a %check section and run 'perl ./tests/run.pl'

- please remove the executable bit from /usr/share/po-debconf/pot-header

- meanwhile nmu2 is out. Please update the package, fix the issues and then let me have a final quick look. I promise it won't take long.

Any progress on this?

now seems that is Oron Peled that not respond,
I can maintain it too . 
we need this one and debhelper to have alien-0.88, seems that we only miss this 2 to close this cycle of packages ...

1. "Mea Culpa" -- I left it stalled for ages, but now that there is renewed
   interest in Debian packaging tools for Fedora, I would really be happy
   to pull this through (if I would be absolved for my previous sins ;-)

2. Updated package (not final yet, see below):
   Spec URL: http://oron.fedorapeople.org/deb-package/po-debconf.spec
   SRPM URL: http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16+nmu2-1.fc18.src.rpm

3. Review items NOT fixed yet:
   * The "+nmu2" string is still part of the version and not the release:
     - It is really part of the upstream Debian versioning
     - So IMO it is not covered by the policy in:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-Numeric_Version_in_Release
     - I was worried about possible version ordering issues with the '+'
       (or in other Debian packages the '~')
       However, it seems we are lucky and RPM rules are similar to Debian's dpkg.
     - Examples:
            $ rpmdev-vercmp 1.0.16-1 "1.0.16+nmu2-1"
            1.0.16-1 < 1.0.16+nmu2-1
            $ rpmdev-vercmp 1.0.16-1 "1.0.16~nmu2-1"
            1.0.16-1 > 1.0.16~nmu2-1

     - Can we stick with the upstream versioning for this?

   * The '%check' does not work yet (errors I have to debug)

(In reply to comment #17)

> 3. Review items NOT fixed yet:
>    * The "+nmu2" string is still part of the version and not the release:
>      - It is really part of the upstream Debian versioning
>      - So IMO it is not covered by the policy in:
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-
> Numeric_Version_in_Release
>      - I was worried about possible version ordering issues with the '+'
>        (or in other Debian packages the '~')
>        However, it seems we are lucky and RPM rules are similar to Debian's
> dpkg.
>      - Examples:
>             $ rpmdev-vercmp 1.0.16-1 "1.0.16+nmu2-1"
>             1.0.16-1 < 1.0.16+nmu2-1
>             $ rpmdev-vercmp 1.0.16-1 "1.0.16~nmu2-1"
>             1.0.16-1 > 1.0.16~nmu2-1
> 
>      - Can we stick with the upstream versioning for this?
> 
>    * The '%check' does not work yet (errors I have to debug)

the fedora-review -b 591389 
is clean, just one warning not blocker 
Rpmlint
-------
Checking: po-debconf-1.0.16+nmu2-1.fc17.noarch.rpm
po-debconf.noarch: W: spelling-error %description -l en_US utils -> tills
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


Numeric version seems correct, source version is indeed 1.0.16+nmu2 nothing we can do about it, so po-debconf-1.0.16+nmu2-1.fc17.noarch.rpm looks good .

Package approved.

I don't know what you what todo about %check 
cd tests ; ./run.pl
tests fails because :
01a-debconf-gettextize-a.pl .. /home/mock_lib/fedora-18-x86_64/root/builddir/build/BUILD/po-debconf-1.0.16+nmu2/debconf-updatepo: line 130: /usr//share/intltool-debian/intltool-update: No such file or directory
so I do as quick workaround :
yum install intltool-0.50.2-3.fc18.noarch
ln -s /usr/bin /usr/share/intltool-debian
ll /usr/share/intltool-debian
lrwxrwxrwx 1 root root 8 Mai  8 05:43 /usr/share/intltool-debian -> /usr/bin
and  ./run.pl
ends with:
Test Summary Report
-------------------
01a-debconf-gettextize-a.pl (Wstat: 0 Tests: 5 Failed: 2)
  Failed tests:  4-5
02-extract-flags.pl        (Wstat: 0 Tests: 4 Failed: 2)
  Failed tests:  2, 4
03-merge-flags.pl          (Wstat: 0 Tests: 6 Failed: 3)
  Failed tests:  2, 4, 6
04-po2debconf.pl           (Wstat: 0 Tests: 7 Failed: 4)
  Failed tests:  2-3, 5, 7
Files=4, Tests=22,  1 wallclock secs ( 0.05 usr  0.01 sys +  0.95 cusr  0.25 csys =  1.26 CPU)
Result: FAIL
Failed 4/4 test programs. 11/22 subtests failed.

(In reply to comment #16)
> I can maintain it too . 

Sergio, if you want to maintain it, you shouldn't review it. It don't understand why you are in a hurry, especially given that there is still something unclear.

Frankly speaking I consider it a very hostile move that you just 'steal' my review and approve the package. We have a policy for stalled reviews [1], please be so kind as to follow it.

(In reply to comment #17)

> 3. Review items NOT fixed yet:
>    * The "+nmu2" string is still part of the version and not the release:
>      - It is really part of the upstream Debian versioning
>      - So IMO it is not covered by the policy in:
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-
> Numeric_Version_in_Release

I disagree. We all agree that the version (or upstream_version as Debian calls it is 1.0.16+nmu2. We don't seem to agree on our naming guidelines.

Generally the guidelines recommend: "If the version is non-numeric (contains tags that are not numbers), you may need to include the additional non-numeric characters in the release field." For post-release packages like this one there are two different groups: "Properly ordered simple versions" (1.0.16a, 1.0.16b, ...) and "human readable" versions. Our package definitely falls into the latter group.

For both groups the guidelines state: "{X} is the release number increment, and %{posttag} is the string that came from the version. Here, the period '.' should be used as the delimiter between the release number increment, and the non-numeric version string. No other extra characters should appear in the Release field.

Even if the current naming with the non-numeric part in the version works, we cannot really be sure it continues to. The next upstream version could be named 1.0.16+abc1 and then we have a problem. We should not use + or ~ in version or release at all. ~ works as of rpm 4.10, but if forbidden as per naming guidelines. Therefor I still think the proper version/release is 1.0.16-1.nmu2.



(In reply to comment #18)
> Numeric version seems correct, source version is indeed 1.0.16+nmu2 nothing
> we can do about it, so po-debconf-1.0.16+nmu2-1.fc17.noarch.rpm looks good .

We *can* do something about it, just as we do for every other package where the source tarball has a non-numeric version.

> I don't know what you what todo about %check 
> cd tests ; ./run.pl
> tests fails because :
> 01a-debconf-gettextize-a.pl ..
> /home/mock_lib/fedora-18-x86_64/root/builddir/build/BUILD/po-debconf-1.0.
> 16+nmu2/debconf-updatepo: line 130:
> /usr//share/intltool-debian/intltool-update: No such file or directory
> so I do as quick workaround :
> yum install intltool-0.50.2-3.fc18.noarch
> ln -s /usr/bin /usr/share/intltool-debian
> ll /usr/share/intltool-debian
> lrwxrwxrwx 1 root root 8 Mai  8 05:43 /usr/share/intltool-debian -> /usr/bin

As a start, we should probably buildrequire intltool and patch the tests to look for intltool-debian in /usr/bin.

Nevertheless I wouldn't call this a blocker, same for the versioning. This is no longer my review, I will not block it but only state my concerns.

> Therefor I still think the proper version/release is 1.0.16-1.nmu2.

+1, this is the safest, sanest, and most compliant way to do it (imo)

(In reply to comment #19)
> (In reply to comment #16)
> > I can maintain it too . 
> 
> Sergio, if you want to maintain it, you shouldn't review it. It don't
> understand why you are in a hurry, especially given that there is still
> something unclear.
> 
> Frankly speaking I consider it a very hostile move that you just 'steal' my
> review and approve the package. We have a policy for stalled reviews [1],
> please be so kind as to follow it.

No, you looks like unresponsive since 2012-06, so I take over , since 2013-01-29 nobody reply and  
2013-04-25 nobody reply 
 
> (In reply to comment #17)
> 
> > 3. Review items NOT fixed yet:
> >    * The "+nmu2" string is still part of the version and not the release:
> >      - It is really part of the upstream Debian versioning
> >      - So IMO it is not covered by the policy in:
> > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-
> > Numeric_Version_in_Release
> 
> I disagree. We all agree that the version (or upstream_version as Debian
> calls it is 1.0.16+nmu2. 

well this is what happens 1.0.16+nmu2 is the source version as I state .
 

> Therefor I still think the proper version/release is
> 1.0.16-1.nmu2.

if source version change to 1.0.16+nmu3 your formula doesn't work.
 
 
> (In reply to comment #18)
> > Numeric version seems correct, source version is indeed 1.0.16+nmu2 nothing
> > we can do about it, so po-debconf-1.0.16+nmu2-1.fc17.noarch.rpm looks good .
> 
> We *can* do something about it, just as we do for every other package where
> the source tarball has a non-numeric version.
> 
> > I don't know what you what todo about %check 
> > cd tests ; ./run.pl
> > tests fails because :
> > 01a-debconf-gettextize-a.pl ..
> > /home/mock_lib/fedora-18-x86_64/root/builddir/build/BUILD/po-debconf-1.0.
> > 16+nmu2/debconf-updatepo: line 130:
> > /usr//share/intltool-debian/intltool-update: No such file or directory
> > so I do as quick workaround :
> > yum install intltool-0.50.2-3.fc18.noarch
> > ln -s /usr/bin /usr/share/intltool-debian
> > ll /usr/share/intltool-debian
> > lrwxrwxrwx 1 root root 8 Mai  8 05:43 /usr/share/intltool-debian -> /usr/bin
> 
> As a start, we should probably buildrequire intltool and patch the tests to
> look for intltool-debian in /usr/bin.
> 
> Nevertheless I wouldn't call this a blocker, same for the versioning. This
> is no longer my review, I will not block it but only state my concerns.

Don't see the need of %check session in build package, so I approved this 

Sorry for any miss understood

(In reply to comment #20)
> > Therefor I still think the proper version/release is 1.0.16-1.nmu2.
> 
> +1, this is the safest, sanest, and most compliant way to do it (imo)

http://ftp.de.debian.org/debian/pool/main/p/po-debconf/ 

we got 
http://ftp.de.debian.org/debian/pool/main/p/po-debconf/po-debconf_1.0.16+nmu1.tar.gz

http://ftp.de.debian.org/debian/pool/main/p/po-debconf/po-debconf_1.0.16+nmu2.tar.gz 

what we do if goes to po-debconf_1.0.16+nmu3 ? 

Can we move forward  ? or this is a blocker like %check session ?

> what we do if goes to po-debconf_1.0.16+nmu3 ? 

Then use 1.0.16-2.nmu3 (which is > than 1.0.16-1.nmu2)

Note, as long as you always increment X in
1.0.16-X.<string>
on subsequent releases, you can guarantee upgrade path.  Please re-read 
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-Numeric_Version_in_Release

if that is still unclear.

In particular,
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Post-Release_packages

(In reply to comment #24)
> In particular,
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Post-
> Release_packages

diff po-debconf.spec.orig po-debconf.spec -up 

-Version:       1.0.16+nmu2
-Release:       1%{?dist}
+Version:       1.0.16
+Release:       1.nmu2%{?dist}

-Source0:       http://ftp.de.debian.org/debian/pool/main/p/%{name}/%{name}_%{version}.tar.gz
+Source0:       http://ftp.de.debian.org/debian/pool/main/p/%{name}/%{name}_%{version}+nmu2.tar.gz

will correct version/release ?

Management:
 * I thought Christoph and Sergio were coordinated about the review switch. Sorry.
 * As I understand it -- I'm the maintainer, so either of you can review.
 * For the record, the last huge review delay was my fault (however, the previous one was Christoph's, so we're even ;-)

Version:
 * I see we got a consensus on the version/release thingie -- fixed and documented
   inside the new .spec file

Self-tests:
 * Sergio and Christoph helped me with %check, I was looking at the wrong errors.
 * The tests can now run (with intltool, etc.), but not all are passing.
 * I made them conditional for now (--with=check) and documented in the .spec

Spec URL: http://oron.fedorapeople.org/deb-package/po-debconf.spec
SRPM URL:
  http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16-1.nmu2.fc18.src.rpm

Unless there is some major foul, I'll move along pushing it to Fedora.

Thank you all for your help and patience.

the review now have 3 problems 

1 - [!]: Spec use %global instead of %define.

just change all %define (s) by %global

2 - incoherent-version-in-changelog 1.0.16+nmu2-1
please change to 1.0.16-1.nmu2 changelog entry 

3 - Diff spec file in url and in SRPM seems to me that po-debconf.spec is not the latest version , so is just update  http://oron.fedorapeople.org/deb-package/po-debconf.spec

1. Fixed to %global
2. Fixed changelog entry and updated its date
3. Rebuilt and uploaded both .spec and .srpm to previous URL's

yours .sprm have an old .spec 

I'll add || : at test and run it (not as optional),but not an issue   

 %check
-( cd ./tests && PODEBCONF_LIB=/usr/bin ./run.pl )
+( cd ./tests && PODEBCONF_LIB=/usr/bin ./run.pl ) || : # ignore failues for now
 %endif
 
After upload the correct po-debconf-1.0.16-1.nmu2.fc18.src.rpm I think, we all agree that package don't have blockers, so is approved and you may proceed with cvs request.

Thanks

SRPM URL:
  http://oron.fedorapeople.org/deb-package/po-debconf-1.0.16-1.nmu2.fc18.src.rpm

New Package SCM Request
=======================
Package Name: po-debconf
Short Description: Tool for managing templates file translations with gettext
Owners: oron
Branches: f18 f19
InitialCC: sergiomb cwickert kanarip

(In reply to comment #29)
> I'll add || : at test and run it (not as optional),but not an issue   

You run tests but ignore the results. Let's add more entropy to the Universe! So, may be neither NOT run tests at all nor NOT ignore the results?

(In reply to comment #31)
> (In reply to comment #29)
> > I'll add || : at test and run it (not as optional),but not an issue   
> 
> You run tests but ignore the results. Let's add more entropy to the
> Universe! So, may be neither NOT run tests at all nor NOT ignore the results?

OK , all fixed , 
But fedora‑cvs should go to "?" , when cvs done who done it change to "+" , so I try change  fedora‑cvs "?" let see if I can

Git done (by process-git-requests).

Oron Peled , you may submit the package ... 

Thanks,

po-debconf-1.0.16-1.nmu2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/po-debconf-1.0.16-1.nmu2.fc18

po-debconf-1.0.16-1.nmu2.fc18 has been pushed to the Fedora 18 testing repository.

po-debconf-1.0.16-1.nmu2.fc18 has been pushed to the Fedora 18 stable repository.

Hi, po-debconf for F19 is not pushed , we need do (please):

fedpkg switch-branch f19
fedpkg update

to submit po-debconf. As you had done with this package for F18 , you could mention this bug number .

Thanks,

po-debconf-1.0.16-1.nmu2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/po-debconf-1.0.16-1.nmu2.fc19

po-debconf-1.0.16-1.nmu2.fc19 has been pushed to the Fedora 19 stable repository.