[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #16 from Fedora Update System  ---
php-gettext-languages-2.1.3-4.fc24 has been pushed to the Fedora 24 stable
repository. If problems still persist, please make note of it in this bug
report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA
Last Closed||2017-02-22 12:23:39



--- Comment #15 from Fedora Update System  ---
php-gettext-languages-2.1.3-4.fc25 has been pushed to the Fedora 25 stable
repository. If problems still persist, please make note of it in this bug
report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #14 from Fedora Update System  ---
php-gettext-languages-2.1.3-4.fc25 has been pushed to the Fedora 25 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-46016c3a74

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #13 from Fedora Update System  ---
php-gettext-languages-2.1.3-4.fc24 has been pushed to the Fedora 24 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-aca77236c7

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Randy Barlow  changed:

   What|Removed |Added

   Fixed In Version||php-gettext-languages-2.1.3
   ||-3.fc26



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #12 from Fedora Update System  ---
php-gettext-languages-2.1.3-3.fc25 has been submitted as an update to Fedora
25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-46016c3a74

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ASSIGNED|MODIFIED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #11 from Fedora Update System  ---
php-gettext-languages-2.1.3-3.fc24 has been submitted as an update to Fedora
24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-aca77236c7

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #10 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/php-gettext-languages

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Remi Collet  changed:

   What|Removed |Added

  Flags||fedora-review+



--- Comment #9 from Remi Collet  ---
1 minor easy fix:

[!]: Rpmlint is run on all installed packages.
php-gettext-languages.src:44: W: rpm-buildroot-usage %prep sed -i
"s:require_once.*:require_once
'%{buildroot}/%{_datadir}/php/Gettext/Languages/autoloa
der.php';:" tests/bootstrap.php
easy fix => move this in %check


No blocker

[x]: Package complies to the Packaging Guidelines

=== APPROVED ===

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Remi Collet  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|nob...@fedoraproject.org|fed...@famillecollet.com



--- Comment #7 from Remi Collet  ---
Created attachment 1247398
  --> https://bugzilla.redhat.com/attachment.cgi?id=1247398=edit
phpci.log

 Note: phpCompatInfo version 5.0.4 DB version 1.17.0 built Jan 24 2017
 09:56:36 CET static analyze results in

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #8 from Remi Collet  ---
Created attachment 1247399
  --> https://bugzilla.redhat.com/attachment.cgi?id=1247399=edit
review.txt

Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1413434
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, PHP, Shell-api

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Remi Collet  changed:

   What|Removed |Added

 CC||rcol...@redhat.com



--- Comment #6 from Remi Collet  ---
As php-gettext-gettext requires php-gettext-languages, on the latest have to
own this directory.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #5 from Randy Barlow  ---
Oh, one more thing: fedora-review is upset that php-gettext-gettext and this
package both own /usr/share/php/Gettext. I don't know a way around that without
creating a php-gettext-common package that just owns that directory, or by
making php-gettext-languages own it (which just seems weird, but maybe that's
OK?) This seems like a problem that might be common with PHP packages - is
there a common solution, or is it something that is just ignored?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #4 from Randy Barlow  ---
Hello Remi! It was wonderful to meet you at DevConf.cz!

(In reply to Remi Collet from comment #3)
> Notice: this means that the comopser.json is incomplete, should have this
> listed in the "bin" section (ex, see
> https://github.com/sebastianbergmann/phpunit/blob/5.7/composer.json#L61).
> 
> => could be submitted upstream

https://github.com/mlocati/cldr-to-gettext-plural-rules/pull/12

> => using "#!/usr/bin/env php" can be submitted upstream.

https://github.com/mlocati/cldr-to-gettext-plural-rules/pull/13

Upstream had a question about whether the shebang will be printed in the output
- if you know the answer it could be helpful to chime in.

> > Do you think we should distribute that executable? 
> 
> yes

Done!


The new code is here:

Spec URL: https://bowlofeggs.fedorapeople.org/php-gettext-languages.spec
SRPM URL:
https://bowlofeggs.fedorapeople.org/php-gettext-languages-2.1.3-3.fc26.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #3 from Remi Collet  ---
> The upstream project already has an autoloader.php

Indeed, I miss it.

> Also, I realized that it seems that they may expect that bin/export.{php,sh} 
> to get distributed with the package 

Indeed (reading the README)

Notice: this means that the comopser.json is incomplete, should have this
listed in the "bin" section (ex, see
https://github.com/sebastianbergmann/phpunit/blob/5.7/composer.json#L61).

=> could be submitted upstream

The .sh is uneeded, (the .php with a propoer shebang is enough)

=> using "#!/usr/bin/env php" can be submitted upstream.

(Fedora Guidelines, recommends #!/usr/bin/php, but using env is also ok, if you
require php-cli, at least this allow the command to work with SCL... even if
this is more correct outside Fedora).

> Do you think we should distribute that executable? 

yes

> If so, should I name it something more specific (gettext-languages-export.sh, 
> cldr-to-gettext-plural-rules-export.sh?) 

Yes. "%{name}-export" seems ok (no suffix)

> but if we want to distribute it I'll need to sed a copy of it instead (and 
> sed the one we install separately).

Indeed. 
Patch it in %prep for installation
Patch it again in %check.

From php-nikic-php-parser

   sed -e 's:%{php_home}:%{buildroot}%{php_home}:' \
  bin/php-parse > bin/php-parse-test

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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



--- Comment #2 from Randy Barlow  ---
Hello Remi!

The upstream project already has an autoloader.php for the installed package,
and they had a tests/bootstrap.php that was close to what we needed but not
quite the same. I ended up using two sed statements to modify the
tests/bootstrap.php and bin/export.php so that they require_once with the
installed path. What do you think about that approach?

Also, I realized that it seems that they may expect that bin/export.{php,sh} to
get distributed with the package and I have not done that so far because the
name "export.php" sounds too general to go into /usr/bin. Do you think we
should distribute that executable? If so, should I name it something more
specific (gettext-languages-export.sh, cldr-to-gettext-plural-rules-export.sh?)
The tests rely on this executable running first to generate some data file (see
tests/bootstrap.php), which is why I used that sed statement, but if we want to
distribute it I'll need to sed a copy of it instead (and sed the one we install
separately).

Here's what I have so far, without distributing that executable:

Spec URL: https://bowlofeggs.fedorapeople.org/php-gettext-languages.spec
SRPM URL:
https://bowlofeggs.fedorapeople.org/php-gettext-languages-2.1.3-2.fc26.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Remi Collet  changed:

   What|Removed |Added

 CC||fed...@famillecollet.com



--- Comment #1 from Remi Collet  ---
Quick notes:

- you've been hit by the ".gitattributes" issue. Upstream remove test suite
from the tarball, so you have to use a git snapshot (see various packages, such
as the php-zendframework-zend-* ones for example). Running test suite is a must
when possible (and allow Koschei monitoring)

- better to add an autoloader (using fedora/autoloader) as composer.json state
this library is PSR64 compliant. Will help you:

   - to run the test suite
   - to use this lib. from other package

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1413434] Review Request: php-gettext-languages - Generate gettext language lists with plural rules

[bugzilla]

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

Randy Barlow  changed:

   What|Removed |Added

 Blocks||1414216




Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1414216
[Bug 1414216] Review Request: php-gettext-gettext - PHP gettext manager
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org