Bug 840109
Summary: | Review Request: php-lessphp - A compiler for LESS written in PHP | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Shawn Iwinski <shawn> | ||||
Component: | Package Review | Assignee: | Remi Collet <fedora> | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | fedora, mrunge, notting, package-review | ||||
Target Milestone: | --- | Flags: | fedora:
fedora-review+
gwync: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2012-11-23 07:45:45 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Shawn Iwinski
2012-07-13 18:38:36 UTC
Could you update to latest version 0.3.8 before I do a formal review ? Some notes - you don't have to compress the generated man page (will be done by rpmbuild, and compression tool could change, one day) - tests are currently installed in /usr/share/php, which is in the default include_path. There is no Guidelines about this, but I think it's a bad practice. Please consider moving to /usr/share/tests/php-lessphp (you need to own /usr/share/tests or requires pear which own this dir) - sed (to add shebang) is a bit hard to read. Isn't simpler to use : sed -e '1i#!/bin/bash\n' -i %{libname}/tests/bootstrap.sh (In reply to comment #1) > Could you update to latest version 0.3.8 before I do a formal review ? Updated > Some notes > > - you don't have to compress the generated man page (will be done by > rpmbuild, and compression tool could change, one day) Removed. I did not know about this. Thanks! > - tests are currently installed in /usr/share/php, which is in the default > include_path. There is no Guidelines about this, but I think it's a bad > practice. > > Please consider moving to /usr/share/tests/php-lessphp > (you need to own /usr/share/tests or requires pear which own this dir) I have tried several ways to do this but I cannot come up with a "clean" solution (i.e. not hacking up the source and/or spec file). I will work with upstream to hopefully come up with a way to do this in a future release. > - sed (to add shebang) is a bit hard to read. > > Isn't simpler to use : > > sed -e '1i#!/bin/bash\n' -i %{libname}/tests/bootstrap.sh Fixed upstream via pull request :) Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-lessphp.spec SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-lessphp-0.3.8-1.fc17.src.rpm Simplified the spec and moved tests. Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-lessphp.spec SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-lessphp-0.3.8-2.fc17.src.rpm Build fails. + help2man --version-option=-v --no-info plessc help2man: can't get `--help' info from plessc Try `--no-discard-stderr' if option outputs to stderr Probably because the package is not installed (plessc command not found) Setting the path seems ok. help2man --version-option='-v' --no-info ./plessc [!]: Package must own all directories that it creates. [!]: Package requires other packages for directories it uses. As said in comment #1 you need to own /usr/share/tests or requires php-pear which own this dir - Fixed man page creation - Added tests directory ownership Spec URL: http://siwinski.fedorapeople.org/rpmbuild/SPECS/php-lessphp.spec SRPM URL: http://siwinski.fedorapeople.org/rpmbuild/SRPMS/php-lessphp-0.3.8-3.fc17.src.rpm Created attachment 647273 [details]
php-lessphp-review.txt
Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-17-x86_64
Command line :/usr/bin/fedora-review -r -n /dev/shm/extras/SRPMS/php-lessphp-0.3.8-3.fc17.src.rpm
=== APPROVED === Thanks for the review and all of the help and patience with this package! New Package SCM Request ======================= Package Name: php-lessphp Short Description: A compiler for LESS written in PHP Owners: siwinski Branches: f17 f18 el6 InitialCC: Git done (by process-git-requests). php-lessphp-0.3.8-3.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/php-lessphp-0.3.8-3.fc18 php-lessphp-0.3.8-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/php-lessphp-0.3.8-3.fc17 php-lessphp-0.3.8-3.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/php-lessphp-0.3.8-3.el6 php-lessphp-0.3.8-3.el6 has been pushed to the Fedora EPEL 6 testing repository. php-lessphp-0.3.8-3.fc18 has been pushed to the Fedora 18 stable repository. php-lessphp-0.3.8-3.fc17 has been pushed to the Fedora 17 stable repository. php-lessphp-0.3.8-3.el6 has been pushed to the Fedora EPEL 6 stable repository. |