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

Re: Bug#943785: RFS: python-pyjsparser (ITP bug #943785)



Hi Nick,

thank you very much for taking the time to review the packaging and
providing such detailed and helpful feedback.

On 10.11.19 00:02, Nick Morrott wrote:
> Thank you for your work in packaging python-pyjsparser. Out of
> curiosity, what are you using to be build your package?

My primary motivation for packaging python-pyjsparser is to be able to
bump the version of Charliecloud:

https://tracker.debian.org/pkg/charliecloud

Version 0.10 of Charliecloud adds new dependencies, including
lark-parser which in turn depends on python-js2py which in turn depends
on python-pyjsparser.

> I checked out the package and built it - your work looks good, and the
> build was successful. I then ran it through lintian, Debian's package
> linting tool which you should get into the habit of using before
> uploading to salsa (and once you're a DM/DD, also before uploading to
> the archive).

I have configured my sbuild setup in such a way that it always runs
lintian after builds. But thanks to your feedback I noticed that I was
running lintian with a too high display threshold such that the only
lintian feedback I received was

"I: Lintian run was successful."

and

"Lintian: pass"

despite of the issues you discovered.

I have tweaked the lintian options in my ~/.sbuildrc now. Hopefully I
will not miss similar issues in the future.

> * d/control
>   - no Rules-Requires-Root field [lintian]

Fixed.

>   - the extended package description should probably include details
> about the library's key features/support [lintian]

Done.

> * d/copyright
>   - the only copyright dates I can see in the source are 2014-2015

Judging from the source files, you are right. Judging from the git
commit history I see commits between 2015 and 2019. What is a better
guideline for copyright years: copyright notices in the source or the
git history? Maybe it would be best to combine both information which
would yield 2014-2019. Once I know what should be used, I will update
the copyright file accordingly.

>   - you can add the Upstream-Contact field to the header

Done.

> * d/rules
>   - pybuild can provide verbose output with "export PYBUILD_VERBOSE=1"

Added.

> * d/upstream/metadata
>   - please add and complete this file - there are plenty of examples
> in the team's salsa project [lintian]

Done.

> * d/watch
>   - +1 for asking upstream to version their releases on github

Thanks! Let's see what happens.

>   - the additional (commented) scaffolding inserted into the file by
> dh-make can be removed to leave only the version line and the URL to
> check [lintian]

The idea behind leaving the Github lines as comment was that I could
easily switch to following Github tags/releases provided that upstream
decides to publish them for future releases. But since the
"<project>" string triggers the lintian complaint and replacing this
placeholder would be just guessing until upstream actually provides
releases on Github, I removed all comments related to this.

This also makes lintian happier. :-)

>   - as this is a version 4 watch file, we can take advantage of new
> variable substitutions for the version and extension fields, so that
> the URL to check would look like:
> 
>     https://pypi.debian.net/pyjsparser/pyjsparser@ANY_VERSION@@ARCHIVE_EXT@

Very convenient variables. Changed accordingly.

> * upstream changelog
>   - there is no upstream changelog

To my knowledge upstream does not provide such a file.

> * documentation/examples
>   - There is no documentation at all, aside from the simple example in
> the README. Perhaps this snippet could be installed as an example when
> installing the package?

Done (although I am not sure whether the way I implemented it is the
recommended way to do it). Suggestions how to improve the code are welcome.

> * tests
>   - there are no tests to run at build time (and therefore no
> autopkgtests to run for continuous integration whenever the package's
> dependencies are updated). Looking at the github repo there seems to
> be a significant testsuite that should really be available in the
> Debian source package to take advantage of autopkgtest. [lintian]

The testsuite available on Github requires js2py which is not in the
Debian archive yet (thus the testsuite introduces a circular dependence:
pyjsparser <-> js2py). js2py is the next package on my to-package list
(see above).

I see two options to move forward:

1. Wait for js2py being packaged and provide autopkgtests for pyjsparser
once js2py is available.

2. Install js2py using pip for the test.

None is really nice in my opinion. Which option do you prefer?

> I hope you find this useful - if you have any questions don't hesitate
> to ask the team (the Python list is visible and archived, IRC may get
> a quicker answer...). I'm a new DD and there are likely some things
> that the more experienced members of the team will notice.

Yes, your feedback was definitely very helpful and allowed me to learn
new things.

Thanks,

Peter


Reply to: