Bug 1271379
| Summary: | Review Request: q-text-as-data - Q - Text as Data | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Michele Baldessari <michele> |
| Component: | Package Review | Assignee: | Zbigniew Jędrzejewski-Szmek <zbyszek> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | pablo.iranzo, package-review, zbyszek |
| Target Milestone: | --- | Flags: | zbyszek:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2016-02-09 20:53:40 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Michele Baldessari
2015-10-13 19:30:42 UTC
Summary should not repeat the name. Something like this instead:
Summary: Direct execution of SQL-like queries on tabular text files
Don't gzip the man page. It will be done automatically by rpm.
Also use q-test-as-data.1.* in %files (not .gz), so there is no need to adjust if the compression scheme ever changes.
Why 'rm -f'? The removal should not fail under any circumstances, so plain 'rm' should be used.
You have with_python3 defined, but it is not used anywhere.
Does the program support python3? If yes, then it should run under python3.
Independently of whether python2 or python3 is used, the shebang line should be changed from /usr/bin/env... to /usr/bin/python2 or /usr/bin/python3. Try:
sed -i '1s/#!.*/#!%{__python2}/' FILENAME
Thanks Zbigniew! (In reply to Zbigniew Jędrzejewski-Szmek from comment #1) > Summary should not repeat the name. Something like this instead: > Summary: Direct execution of SQL-like queries on tabular text files ok, done > Don't gzip the man page. It will be done automatically by rpm. > Also use q-test-as-data.1.* in %files (not .gz), so there is no need to > adjust if the compression scheme ever changes. ok, done > Why 'rm -f'? The removal should not fail under any circumstances, so plain > 'rm' should be used. right, done > You have with_python3 defined, but it is not used anywhere. > > Does the program support python3? If yes, then it should run under python3. > > Independently of whether python2 or python3 is used, the shebang line should > be changed from /usr/bin/env... to /usr/bin/python2 or /usr/bin/python3. Try: > sed -i '1s/#!.*/#!%{__python2}/' FILENAME So I have added python3 support in the package, so we just need to flip the switch once python3 support lands (I made a note of which github issue tracks this in the spec file). Spec URL: http://acksyn.org/files/rpms/q-data-as-text/q-text-as-data.spec SRPM URL: http://acksyn.org/files/rpms/q-data-as-text/q-text-as-data-1.5.0-2.fc23.src.rpm Thanks again Please note that this spec file will not build if with_pytohn3 is undefined (e.g. on RHEL). "%if %{?with_python3}" would expand to "%if", which is not valid. It works for Fedora though, so as far as this review goes, that's enough.
"%if 0%{?with_python3}" would make more sense, and then there's no need to define with_python3 to 0, just define it to 1 in the future.
+ license is acceptable (GPLv3)
+ license file is present, %license is used
+ provides and requires look OK
+ builds and installs fine
+ name is ok
+ latest version
+ no scriptlets present or necessary
Package is APPROVED.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/q-text-as-data q-text-as-data-1.5.0-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-0c4c01223d q-text-as-data-1.5.0-2.fc23 has been pushed to the Fedora 23 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-2016-0c4c01223d q-text-as-data-1.5.0-2.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report. |