[Bug 1286867] Review Request: python-os_virtual_interfacesv2_python_novaclient_ext - Adds Virtual Interfaces support to python-novaclient

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Christos Triantafyllidis <christos.triantafyllidis@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|christos.triantafyllidis@gm |nobody@xxxxxxxxxxxxxxxxx
                   |ail.com                     |
              Flags|fedora-review-              |



--- Comment #5 from Christos Triantafyllidis <christos.triantafyllidis@xxxxxxxxx> ---
Hello Paul,

Thank you very much for the review.

On your comments:
- Missing LICENSE file, this is pretty big and will need to get resolved
upstream.

First of all I don't think that is a blocker, is it? It is on the SHOULDs.
Nevertheless I totally agree with you and that is why I raised it as pull
request to upstream when I submitted this review request:
https://github.com/rackerlabs/os_virtual_interfacesv2_ext/pull/3
The package will be rebuild as soon as that is merged.


- Missing python3 support. While the openstack libraries don't all support it,
it would be nice to see the packaging here for the future.  Refer to the python
packaging guidelines.

I actually copied the template of python packaging guidelines. Unfortunately
the dependencies of that don't support python3 yet thus I removed the python3
part from the spec file. Is it needed and commented out? As soon as
python-novaclient is available in python3 package I'll post an update.

- No need for global sum, you can use %summary
As above I just used what is in the template for multiversion python packages
in the python packaging guidelines. I'll try setting Summary on the main
package and using %summary in the python2 subpackage and if that works I'm
happy to switch to it.

- rpmlint errors.
Those refer to the word "novaclient" not being in en_US dictionary. As this is
a package name I don't think we should change it.

- Missing documentation.
Given that there is no upstream documentation I'm not sure if I should write
and include my own on. I'm happy to raise it as an issue to upstream and
contribute to upstream but till/unless it is accepted I'd prefer to not include
any.

- 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 %license.
Given that source package doesn't include the text of the license in its own
file, that can't be included.

- Spec file is legible and written in American English.
I suspect that refers to the rpmlint errors. I'd say it is safe to ignore.


I'm putting it back in the queue of NEW packages feel free to take the ticket
if you want to do an official review (I see that the manual review parts are
missing so I assume you were not doing an official review).

Cheers,
Christos

PS: Removing the fedora-review (-) flag as that should be used only if the
package is not suitable for packaging in Fedora (for legal or other reasons), a
review that needs works does need to have it.

-- 
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@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]