Bug 847777
| Summary: | Review Request: strata-sdk - Python library for Red Hat customer portal's RESTful service interface | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Keith Robertson <kroberts> |
| Component: | Package Review | Assignee: | Bohuslav "Slavek" Kabrda <bkabrda> |
| Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | bkabrda, jmoran, package-review |
| Target Milestone: | --- | Flags: | bkabrda:
fedora-review?
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2015-07-02 09:48:14 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: | |||
|
Description
Keith Robertson
2012-08-13 13:56:13 UTC
I will take this for a review. - BuildRequires: "python-setuptools-devel" should be just "python-setuptools". - Your Requies: python-lxml should specify the version as in setup.py (>= 2.2.8). - Requires: python is useless, because the automatic dependency processor will pick that dependency up and specify it better: $ rpm -q -p --requires strata-sdk-1.0.1-0.fc19.noarch.rpm ... python(abi) = 2.7 ... So please drop the Requires: python line. - Is there a specific reason to run build and install the way you do and not doing just "python setup.py [build|install]"? If not, please use the standard way, as the current state is somehow confusing. - When there are tests present in the package, it is a good practice to run them in the %check section of the specfile. This ensures that the package works as deployed from source. Please do this. - The first release should be 1, not 0. - I'm not sure about the URL you provide. Typically, it should point to a page with some kind of the information about the project - I found no information about strata-sdk at your URL. (In reply to comment #2) > - BuildRequires: "python-setuptools-devel" should be just > "python-setuptools". Done > - Your Requies: python-lxml should specify the version as in setup.py (>= > 2.2.8). Done > - Requires: python is useless, because the automatic dependency processor > will pick that dependency up and specify it better: > > $ rpm -q -p --requires strata-sdk-1.0.1-0.fc19.noarch.rpm > ... > python(abi) = 2.7 > ... > > So please drop the Requires: python line. Dropped > > - Is there a specific reason to run build and install the way you do and not > doing just "python setup.py [build|install]"? If not, please use the > standard way, as the current state is somehow confusing. > - When there are tests present in the package, it is a good practice to run > them in the %check section of the specfile. This ensures that the package > works as deployed from source. Please do this. The tests need an internet connection to run, as such, it really doesn't make sense to execute them. > - The first release should be 1, not 0. > - I'm not sure about the URL you provide. Typically, it should point to a > page with some kind of the information about the project - I found no > information about strata-sdk at your URL. I will open a ticket and get a fedorahosted project. Updated spec file: http://kojak.fedorapeople.org/strata-sdk.spec Updated SRPM: http://kojak.fedorapeople.org/strata-sdk-1.0.0-0.fc17.src.rpm SRC tarball for above: http://kojak.fedorapeople.org/strata-sdk-1.0.0.tar.gz I'll update the spec with a fedorahosted project as soon as I get one created. cheers (In reply to comment #5) > (In reply to comment #2) > > - BuildRequires: "python-setuptools-devel" should be just > > "python-setuptools". > Done Hmm, the specfile you are referring to still has python-setuptools-devel. > > - Your Requies: python-lxml should specify the version as in setup.py (>= > > 2.2.8). > Done > > - Requires: python is useless, because the automatic dependency processor > > will pick that dependency up and specify it better: > > > > $ rpm -q -p --requires strata-sdk-1.0.1-0.fc19.noarch.rpm > > ... > > python(abi) = 2.7 > > ... > > > > So please drop the Requires: python line. > Dropped > > > > - Is there a specific reason to run build and install the way you do and not > > doing just "python setup.py [build|install]"? If not, please use the > > standard way, as the current state is somehow confusing. > > - When there are tests present in the package, it is a good practice to run > > them in the %check section of the specfile. This ensures that the package > > works as deployed from source. Please do this. > The tests need an internet connection to run, as such, it really doesn't > make sense to execute them. > Ok, agreed. > > - The first release should be 1, not 0. The specfile still has release 0. Please fix this. > > - I'm not sure about the URL you provide. Typically, it should point to a > > page with some kind of the information about the project - I found no > > information about strata-sdk at your URL. > I will open a ticket and get a fedorahosted project. > > > Updated spec file: http://kojak.fedorapeople.org/strata-sdk.spec > Updated SRPM: http://kojak.fedorapeople.org/strata-sdk-1.0.0-0.fc17.src.rpm > SRC tarball for above: http://kojak.fedorapeople.org/strata-sdk-1.0.0.tar.gz When doing changes during review, it is customary to bump the changelog and sum up all the changes you do in a new changelog entry (=new release). Please do so for any future changes. Thanks. When you correct the minor issues mentioned above and have the URL, please post a new version of SPEC and SRPM, I believe that I will approve them then. Since there seems to be no progress here, I'm closing this bug. Feel free to reopen if you wish to restart the review. |