Bug 1409654

any reason not to take archive from github?

Not sure how it analyzing it, but I think that you should provide binaries for both py2 and py3.

(In reply to Igor Gnatenko from comment #1)
> any reason not to take archive from github?

Hmm... I thought GitHub Releases page only contained git tags so I chose to use the release on PyPI to follow [1]:

"If the upstream does create tarballs you should use them as tarballs provide an easier trail for people auditing the packages."

I see now that the same zip source archive is also hosted on GitHub. I guess both, PyPI and GitHub, are equivalent now or you think GitHub should be preferred?

> Not sure how it analyzing it, but I think that you should provide binaries
> for both py2 and py3.

What are the arguments for providing binaries for both versions of Python?

The Packaging Guidelines for Python says:

"If the executables provide the same functionality independent of whether they are run on top of Python 2 or Python 3, then only the Python 3 version of the executable should be packaged." [2]

I think pydocstyle running on Python 3 will happily check Python 2 source code docstrings.

BTW, are you volunteering for a review ;-)?

[1] https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Hosting_Services
[2] https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.2Fbin

(In reply to Tadej Janež from comment #2)
> (In reply to Igor Gnatenko from comment #1)
> > any reason not to take archive from github?
> 
> Hmm... I thought GitHub Releases page only contained git tags so I chose to
> use the release on PyPI to follow [1]:
> 
> "If the upstream does create tarballs you should use them as tarballs
> provide an easier trail for people auditing the packages."
> 
> I see now that the same zip source archive is also hosted on GitHub. I guess
> both, PyPI and GitHub, are equivalent now or you think GitHub should be
> preferred?
GitHub should be preferred as it's *real* source (with licenses, tests and all stuff). PyPI tarball is pre-processed source (usually without license, tests).
> 
> > Not sure how it analyzing it, but I think that you should provide binaries
> > for both py2 and py3.
> 
> What are the arguments for providing binaries for both versions of Python?
> 
> The Packaging Guidelines for Python says:
> 
> "If the executables provide the same functionality independent of whether
> they are run on top of Python 2 or Python 3, then only the Python 3 version
> of the executable should be packaged." [2]
> 
> I think pydocstyle running on Python 3 will happily check Python 2 source
> code docstrings.
Question whether if it will check py2 source code docstrings properly. E.g. you have py3-incompatible source.
> 
> BTW, are you volunteering for a review ;-)?
If you are going to change source to github - then definitely yes ;)
> 
> [1] https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Hosting_Services
> [2]
> https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.2Fbin

(In reply to Igor Gnatenko from comment #3)
> (In reply to Tadej Janež from comment #2)
> GitHub should be preferred as it's *real* source (with licenses, tests and
> all stuff). PyPI tarball is pre-processed source (usually without license,
> tests).

I disagree. The PyPI tarball is the actual upstream release tarball, whereas the GitHub's Source archive link [1] is just a tarball of everything that is in the git repo.
(As a side-note, pydocstyle actually uses GitHub's Releases feature and they provide the same release tarball there [2] as is available on PyPI [3].)

I'm sure you know that in the Python world, source distributions are usually made with 'python setup.py sdist' command. And application authors can control what gets included in the source distribution by specifying it in setup.py and MANIFEST.in files.

In pydocstyle's case, the authors decided to include the LICENSE file in the source distribution (LICENSE-MIT) but they omitted the tests (src/tests). I think tests are valuable and should be included in the source distribution.
However, I think I should try to persuade the upstream to include them, not side-step them by using the GitHub's Source archive link.

> Question whether if it will check py2 source code docstrings properly. E.g.
> you have py3-incompatible source.

I checked this with Django 1.4 which is a quite large Python 2 project without Python 3 support and it works without a problem.

Here are the details:

[tadej@tlinux64 pydocstyle-for-python2-only-code]$ wget -q https://www.djangoproject.com/download/1.4/tarball -O Django-1.4.tar.gz
[tadej@tlinux64 pydocstyle-for-python2-only-code]$ tar -xf Django-1.4.tar.gz 
[tadej@tlinux64 pydocstyle-for-python2-only-code]$ cd Django-1.4/
[tadej@tlinux64 Django-1.4]$ cat $(which pydocstyle)
#!/usr/bin/python3
# EASY-INSTALL-ENTRY-SCRIPT: 'pydocstyle==1.1.1','console_scripts','pydocstyle'
__requires__ = 'pydocstyle==1.1.1'
import sys
from pkg_resources import load_entry_point

if __name__ == '__main__':
    sys.exit(
        load_entry_point('pydocstyle==1.1.1', 'console_scripts', 'pydocstyle')()
    )
[tadej@tlinux64 Django-1.4]$ pydocstyle django
django/contrib/admin/validation.py WARNING: __all__ is defined as a list, this means pydocstyle cannot reliably detect contents of the __all__ variable, because it can be mutated. Change __all__ to be an (immutable) tuple, to remove this warning. Note, pydocstyle uses __all__ to detect which definitions are public, to warn if public definitions are missing docstrings. If __all__ is a (mutable) list, pydocstyle cannot reliably assume its contents. pydocstyle will proceed assuming __all__ is not mutated.
django/contrib/gis/measure.py WARNING: __all__ is defined as a list, this means pydocstyle cannot reliably detect contents of the __all__ variable, because it can be mutated. Change __all__ to be an (immutable) tuple, to remove this warning. Note, pydocstyle uses __all__ to detect which definitions are public, to warn if public definitions are missing docstrings. If __all__ is a (mutable) list, pydocstyle cannot reliably assume its contents. pydocstyle will proceed assuming __all__ is not mutated.
[ ... output trimmed ... ]
django/__init__.py:1 at module level:
        D104: Missing docstring in public package
django/__init__.py:3 in public function `get_version`:
        D401: First line should be in imperative mood ('Derive', not 'Derives')
django/bin/django-admin.py:1 at module level:
        D100: Missing docstring in public module
[ ...  output trimmed ... ]
django/middleware/common.py:148 in private function `_is_internal_request`:
        D401: First line should be in imperative mood ('Return', not 'Returns')
django/middleware/__init__.py:1 at module level:
        D104: Missing docstring in public package
[tadej@tlinux64 Django-1.4]$

> > 
> > BTW, are you volunteering for a review ;-)?
> If you are going to change source to github - then definitely yes ;)

I can change it to https://github.com/PyCQA/pydocstyle/releases/download/1.1.1/pydocstyle-1.1.1.zip :-)


[1] https://github.com/PyCQA/pydocstyle/archive/1.1.1.tar.gz
[2] https://github.com/PyCQA/pydocstyle/releases/download/1.1.1/pydocstyle-1.1.1.zip
[3] https://files.pythonhosted.org/packages/source/p/pydocstyle/pydocstyle-1.1.1.zip

Hi,

In this case I have to agree with Igor, it would be better to use the GitHub source. It contains tests and all files have Unix line endings (conversion unnecessary). Use the tarball and not the zip archive.
Correct source URL:
Source0:  https://github.com/PyCQA/%{srcname}/archive/%{version}/%{srcname}-%{version}.tar.gz

Things like removing shebang from a file or dos2unix should be done in the %prep section.

What's up with the Release tag (0%{?dist}.1)?
Should be:
Release:  1%{?dist}

I don't have strong feelings about which source is used, though I do think using the GitHub link is better since it will give you the tests and those can be handy in Koschei.

However, I also don't want to override Igor or fszymanski. I would like to use this package, so if you are willing to use the GitHub link to satisfy them I'll do the review for you.

Igor, Randy,

I've changed the source to GitHub archive download service until upstream includes tests in the release tarballs. I'll submit a pull request for that in near future.

With that I was able to enable tests in %check (with some minor source code modifications).

Regarding fszymanski comment about the release tag, I prefer to use 0%{?dist}.1, 0%{?dist}.2, ... during review phase. Of course, I will change it to 1%{?dist} after the package is accepted and commited into dist-git.

Here is the updated package:
Spec URL: https://raw.githubusercontent.com/tjanez/pydocstyle-package/3456fe8/python-pydocstyle.spec
SRPM URL: https://tadej.fedorapeople.org/python-pydocstyle-1.1.1-0.fc27.2.src.rpm

I would be very happy if any of you do the review.

(In reply to Tadej Janež from comment #7)
> 
> I've changed the source to GitHub archive download service until upstream
> includes tests in the release tarballs. I'll submit a pull request for that
> in near future.

Pull request to include documentation and tests in source distribution has been submitted: https://github.com/PyCQA/pydocstyle/pull/252

I also wanted to update pydocstyle to version 2.0.0 but encountered an issue where the git tag for version 2.0.0 references different code than the code released on PyPI: https://github.com/PyCQA/pydocstyle/issues/251.

So, this affirms my concerns that GitHub tags are not the same as *official* releases on PyPI.

Igor, Randy,

I would be very happy if any of you could do the review.

Igor, Randy,

I've updated pydocstyle to version 2.0.0.

The issue where the git tag for version 2.0.0 referenced different code than the code released on PyPI (https://github.com/PyCQA/pydocstyle/issues/251) was resolved by upstream.

Here is the updated package:
Spec URL: https://raw.githubusercontent.com/tjanez/pydocstyle-package/0e49514/python-pydocstyle.spec
SRPM URL: https://tadej.fedorapeople.org/python-pydocstyle-2.0.0-0.fc27.1.src.rpm

I would be very happy if any of you would do the review.

Created attachment 1284508 [details]
review.txt

Just adding myself to the CC list as an interested user, as I've found pydocstyle handy in the past, and I appreciate the effort to add it to Fedora.

Hello!

This is just a reminder that this package is approved and ready to be added to Fedora. Thanks!

Hi Randy,

(In reply to Randy Barlow from comment #12)
> 
> This is just a reminder that this package is approved and ready to be added
> to Fedora. Thanks!

thanks for the review and the reminder!

I'll add this to Fedora ASAP.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-pydocstyle

python-pydocstyle-2.0.0-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-2b1a47a1ae

python-pydocstyle-2.0.0-1.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-2b1a47a1ae

python-pydocstyle-2.0.0-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-4db0e57f0b

python-pydocstyle-2.0.0-1.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

python-pydocstyle-2.0.0-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.