On Wed, Feb 17, 2021 at 12:28:22PM -0500, John Snow wrote: > On 2/16/21 10:02 PM, Cleber Rosa wrote: > > On Tue, Feb 16, 2021 at 09:59:47PM -0500, Cleber Rosa wrote: > > > On Thu, Feb 11, 2021 at 01:58:40PM -0500, John Snow wrote: > > > > pipenv is a tool used for managing virtual environments with pinned, > > > > explicit dependencies. It is used for precisely recreating python > > > > virtual environments. > > > > > > > > pipenv uses two files to do this: > > > > > > > > (1) Pipfile, which is similar in purpose and scope to what setup.py > > > > lists. It specifies the requisite minimum to get a functional > > > > environment for using this package. > > > > > > > > (2) Pipfile.lock, which is similar in purpose to `pip freeze > > > > > requirements.txt`. It specifies a canonical virtual environment used for > > > > deployment or testing. This ensures that all users have repeatable > > > > results. > > > > > > > > The primary benefit of using this tool is to ensure repeatable CI > > > > results with a known set of packages. Although I endeavor to support as > > > > many versions as I can, the fluid nature of the Python toolchain often > > > > means tailoring code for fairly specific versions. > > > > > > > > Note that pipenv is *not* required to install or use this module; this > > > > is > > > > purely for the sake of repeatable testing by CI or developers. > > > > > > > > Here, a "blank" pipfile is added with no dependencies, but specifies > > > > Python 3.6 for the virtual environment. > > > > > > > > Pipfile will specify our version minimums, while Pipfile.lock specifies > > > > an exact loudout of packages that were known to operate correctly. This > > > > > > Layout? Loadout? > > > > > > > latter file provides the real value for easy setup of container images > > > > and CI environments. > > > > > > > > Signed-off-by: John Snow <js...@redhat.com> > > > > --- > > > > python/Pipfile | 11 +++++++++++ > > > > 1 file changed, 11 insertions(+) > > > > create mode 100644 python/Pipfile > > > > > > > > > > Other than that, > > > > > > Reviewed-by: Cleber Rosa <cr...@redhat.com> > > > > Actually, just one suggestion: bump the position of this patch twice. > > It makes it easier to understand its purpose if it is placed right > > before the "python: add pylint to pipenv" patch. > > > > Cheers, > > - Cleber. > > > > The way the series is laid out is: > > 01-02: pre-requisite fixes > 03-07: Create the package, readmes, etc. > 08: Pipenv support > 09-11: Pylint > 12-13: flake8 > 14-15: mypy > 16-17: isort > 18-20: Testing and pre-requisites > 21-23: Polish > 24: CI support > > Moving the pipenv patch to just before the final pylint patch works OK, but > breaks up the pylint section. Should I still do it? >
OK, now with that approach of groupping in min, it sounds reasonable. My previous point was that pipenv is not needed until right before #10, but that's just nitpicking and almost bikeshedding (I won't admit it easily). > --js > > > (Hm, by this layout, I should probably actually move the pylint fix in #01 > down to appear after the pipenv patch. I could also move the flake8 fixes in > #21 up to be near the other flake8 patches.) Sounds more consistent. Cheers, - Cleber.
signature.asc
Description: PGP signature