Bug 1385244 - Review Request: pew - Manage multiple Python virtual environments
Summary: Review Request: pew - Manage multiple Python virtual environments
Keywords:
Status: CLOSED DUPLICATE of bug 1525570
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1385237 1385240
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-10-15 15:51 UTC by Mathieu Bridon
Modified: 2017-12-19 16:19 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-12-19 16:19:19 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Mathieu Bridon 2016-10-15 15:51:08 UTC
Spec URL: https://bochecha.fedorapeople.org/packages/pew.spec
SRPM URL: https://bochecha.fedorapeople.org/packages/pew-0.1.24-1.fc24.src.rpm

Description:
Python Env Wrapper is a set of commands to manage multiple virtual
environments. Pew can create, delete and copy your environments, using a
single command to switch to them wherever you are, while keeping them in a
single (configurable) location.

Virtualenvs make it easier to work on more than one project at a time without
introducing conflicts in their dependencies.

Pew is completely shell-agnostic and thus works on bash, zsh, fish,
powershell, etc.


Fedora Account System Username: bochecha

Comment 1 VincentS 2017-03-03 17:13:00 UTC
I looked at your review request and I have some comments about it.

I noted there is no license file, you may tell to upstream to add it.

The second thing, the compilation fails when I try to build the SRPM in mock.
Here is the error: 
No matching package to install: 'python3-pythonz-bd >= 1.11.2'
Not all dependencies satisfied
Error: Some packages could not be found.

According to upstream requirements, pythonz-bd for Python3 has still the same name: pythonz-bd.
https://github.com/berdario/pew/blob/master/requirements.txt

Comment 2 Mathieu Bridon 2017-03-03 17:21:27 UTC
> The second thing, the compilation fails when I try to build the SRPM in mock.

I know, I added pythonz-bd to Fedora this afternoon. :)

It should be in Rawhide and F26 already (or tomorrow), and it should be in F25 testing very soon.

Comment 3 VincentS 2017-03-03 20:47:46 UTC
Ok, I tried this and it works. =)

Comment 4 Mathieu Bridon 2017-05-20 18:10:12 UTC
My apologies for not coming back to this sooner. Especially since you were trying to review this as part of your sponsorship process Vincent. :(

Here is a new package.

Spec URL: https://bochecha.fedorapeople.org/packages/pew.spec
SRPM URL: https://bochecha.fedorapeople.org/packages/pew-0.1.26-1.fc25.src.rpm

This is a new upstream version, and I added the README.md file (which wasn't included in the 0.1.24 tarball) as %doc. This file contains a copy of the MIT license, so hopefully that should satisfy the legal requirements.

Comment 5 VincentS 2017-06-15 22:53:07 UTC
Don't worry Mathieu, so here is an informal review for pew package.

REVIEW:

+ OK
- NA 
X ISSUE

+ Package meets naming and packaging guidelines.
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
+ License
+ License field in spec matches
+ License file included in package

+ Spec in American English
+ Spec is legible.
+ Sources match upstream sha256sum:
$ sha256sum -b ~/pew-0.1.26.tar.gz ~/rpmbuild/SOURCES/pew-0.1.26.tar.gz
0e52051393777ebf93b6ed883a049b615e0555978a578b682043f69c7ea8a6bb */opt/builder//pew-0.1.26.tar.gz
0e52051393777ebf93b6ed883a049b615e0555978a578b682043f69c7ea8a6bb */opt/builder//rpmbuild/SOURCES/pew-0.1.26.tar.gz

- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

- Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
X Rpmlint output: 3 errors.
$ rpmlint SPECS/pew.spec /var/lib/mock/fedora-26-x86_64/result/pew-0.1.26-1.fc26.src.rpm /var/lib/mock/fedora-26-x86_64/result/pew-0.1.26-1.fc26.noarch.rpm 
SPECS/pew.spec:12: W: unversioned-explicit-provides python-%{name}
SPECS/pew.spec:13: W: unversioned-explicit-provides python3-%{name}
pew.src: W: spelling-error %description -l en_US Virtualenvs -> Virtual
pew.src: W: spelling-error %description -l en_US zsh -> sh, ssh, ash
pew.src: W: spelling-error %description -l en_US powershell -> power shell, power-shell, powers hell
pew.src:12: W: unversioned-explicit-provides python-%{name}
pew.src:13: W: unversioned-explicit-provides python3-%{name}
pew.noarch: W: spelling-error %description -l en_US Virtualenvs -> Virtual
pew.noarch: W: spelling-error %description -l en_US zsh -> sh, ssh, ash
pew.noarch: W: spelling-error %description -l en_US powershell -> power shell, power-shell, powers hell
pew.noarch: E: wrong-script-interpreter /usr/lib/python3.6/site-packages/pew/pew.py /usr/bin/env python
pew.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/pew/pew.py 644 /usr/bin/env python
pew.noarch: E: wrong-script-interpreter /usr/lib/python3.6/site-packages/pew/shell_config/complete_deploy /usr/bin/env python
pew.noarch: W: no-manual-page-for-binary pew
2 packages and 1 specfiles checked; 3 errors, 11 warnings.

+ final provides and requires are sane.


SHOULD Items:

+ Should build in mock.
+ Should build on all supported archs
- Should function as described.
- Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues :
- I think, I remember that it's forbidden to use this shebang in Fedora package : /usr/bin/env python

For me, you may just resolve rpmlint issues otherwise all the rest is ok.

Comment 6 Mathieu Bridon 2017-06-17 07:17:04 UTC
Thanks for the detailed review Vincent.

Concerning those shebangs, the first one should get removed https://github.com/berdario/pew/pull/141

And I have a feeling the second one also will: https://github.com/berdario/pew/issues/142

Comment 7 Mathieu Bridon 2017-10-26 09:25:42 UTC
Hey, sorry for the long silence!

Here's a new package, which should fix the two rpmlint issues.

Spec URL: https://bochecha.fedorapeople.org/packages/pew.spec
SRPM URL: https://bochecha.fedorapeople.org/packages/pew-1.1.0-1.fc26.src.rpm

Comment 8 Felix Schwarz 2017-12-14 22:02:34 UTC
seems like bug 1525570 is a review request for the same package?

Comment 9 Miro Hrončok 2017-12-19 16:19:19 UTC
Closing this one as per discussion in bug 1525570.

*** This bug has been marked as a duplicate of bug 1525570 ***


Note You need to log in before you can comment on or make changes to this bug.