Subject: Re: bug#26074: [PATCH 1/3] gnu: Add python-htmlmin

From: Catonano <[email-masked]>
Date: Sun, 12 Mar 2017 19:40:38 +0100

Marius,

thank you for reviewing !

2017-03-12 19:28 GMT+01:00 Marius Bakke <[email-masked]>:

> Catonano <[email-masked]> writes:
>
> > * gnu/packages/python.scm (python-flask-login): New variable.
>
>

> +    (arguments
> > +     `(#:tests? #f))
> > +    ;starting phase `check'
> > +    ;running "python setup.py" with command "test" and parameters ()
> > +    ;make: *** No rule to make target 'check'.  Stop.
> > +    ;phase `check' failed after 0.5 seconds
>
> Some projects remove tests before uploading to PyPi which seems to be
> the case here. :(
>
> Can you try downloading the source from
> https://github.com/maxcountryman/flask-login/releases instead?
>

Sure, thanks for the suggestion !

>
> > +    (build-system python-build-system)
> > +    (home-page
> > +     "https://github.com/maxcountryman/flask-login")
> > +    (synopsis "User session management for Flask")
> > +    (description "Flask does not provide user session management
> facilities so you are required to either roll your own or use an
> extension.")
>
> This line is too long (`./pre-inst-env guix lint python-flask-login`
> should warn about this).
>

I just realized that I hadn't edited a description so I did that after
having run the linter. I didn't think to run it again.
Sorry

>
> > +    (license license:expat)))
>
> Other than this the three patches LGTM, except for one detail: we
> normally add python2 variants when possible, like so:
>
> (define-public python2-flask-login
>   (package-with-python2 python-flask-login))
>
> Can you add that for all three packages and send an updated series?
> Please also add a copyright line for yourself at the top of the file.
> You can create a single attachment with all patches with this command:
> `git format-patch -n origin/master --stdout > series.patch`.
>
> Thanks in advance!
>

Sure, thanks for your suggestions !