Bug 1811410

I am taking this review just as I did for #1811409 and #1811377.

Could you, please, update the url's for spec and srpm since they are not valid anymore.

Spec URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01314096-mkdocs/mkdocs.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01314096-mkdocs/mkdocs-1.1-2.fc33.src.rpm

Changes:
- Requires python3dist(lunr) python3dist(nltk)
- Obsoletes mkdocs-basic-theme

(In reply to José Matos from comment #1)
> I am taking this review just as I did for #1811409 and #1811377.
> 
> Could you, please, update the url's for spec and srpm since they are not
> valid anymore.

Thanks! And all the requirement of this package has been satisfied in Rawhide.

(In reply to Robin Lee from comment #2)
> 
> Changes:
> - Requires python3dist(lunr) python3dist(nltk)

These are not necessary. They are built automatically:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_dependencies

> - Obsoletes mkdocs-basic-theme

The other topic that could be addressed is the the RobotSlab font. A part of the fonts are already in Fedora.
python-sphinx_rtd_theme has an example where the part that is already packaged in Fedora is symlinked.

See:
https://src.fedoraproject.org/rpms/python-sphinx_rtd_theme/blob/master/f/python-sphinx_rtd_theme.spec

Other than that the package is in good shape. :-)

Spec URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01315366-mkdocs/mkdocs.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01315366-mkdocs/mkdocs-1.1-3.fc33.src.rpm

Changes:
Add symlinks to the Roboto fonts.

(In reply to José Matos from comment #3)
> (In reply to Robin Lee from comment #2)
> > 
> > Changes:
> > - Requires python3dist(lunr) python3dist(nltk)
> 
> These are not necessary. They are built automatically:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
> #_dependencies

These requirements are not automatically generated.

(In reply to Robin Lee from comment #4)
> > These are not necessary. They are built automatically:
> > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
> > #_dependencies
> 
> These requirements are not automatically generated.

python3dist(lunr) is generated:

Requires
--------
mkdocs (rpmlib, GLIBC filtered):
    /usr/bin/python3
    bootswatch-fonts
    fontawesome-fonts
    fontawesome-fonts-web
    google-roboto-slab-fonts
    js-jquery1
    js-jquery2
    lato-fonts
    python(abi)
    python3.8dist(click)
    python3.8dist(jinja2)
    python3.8dist(livereload)
    python3.8dist(lunr)
    python3.8dist(markdown)
    python3.8dist(pyyaml)
    python3.8dist(setuptools)
    python3.8dist(tornado)
    python3dist(lunr)
    python3dist(mdx-gh-links)
    python3dist(nltk)

Notice as lunr appears above.


Regarding nltk I am curious why it is a dependency of mkdocs, it is not mentioned in the documentation and it does not show in the code.


BTW the License should be:

License: BSD and Tumbolia

The Tumbolia appears because of file mkdocs-1.1/mkdocs/utils/ghp_import.py

Spec URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01315964-mkdocs/mkdocs.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01315964-mkdocs/mkdocs-1.1-4.fc33.src.rpm

Changes:
- Drop explicit lunr requirement
- License specified to BSD and Tumbolia


(In reply to José Matos from comment #5)
> (In reply to Robin Lee from comment #4)
> > > These are not necessary. They are built automatically:
> > > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
> > > #_dependencies
> > 
> > These requirements are not automatically generated.
> 
> python3dist(lunr) is generated:
> 
> Requires
> --------
> mkdocs (rpmlib, GLIBC filtered):
>     /usr/bin/python3
>     bootswatch-fonts
>     fontawesome-fonts
>     fontawesome-fonts-web
>     google-roboto-slab-fonts
>     js-jquery1
>     js-jquery2
>     lato-fonts
>     python(abi)
>     python3.8dist(click)
>     python3.8dist(jinja2)
>     python3.8dist(livereload)
>     python3.8dist(lunr)
>     python3.8dist(markdown)
>     python3.8dist(pyyaml)
>     python3.8dist(setuptools)
>     python3.8dist(tornado)
>     python3dist(lunr)
>     python3dist(mdx-gh-links)
>     python3dist(nltk)
> 
> Notice as lunr appears above.
Fixed
> 
> 
> Regarding nltk I am curious why it is a dependency of mkdocs, it is not
> mentioned in the documentation and it does not show in the code.
Mkdocs requires an optional feature of lunr. And that option feature of lunr requires nltk.
> 
> 
> BTW the License should be:
> 
> License: BSD and Tumbolia
> 
> The Tumbolia appears because of file mkdocs-1.1/mkdocs/utils/ghp_import.py
Fixed.

Thank you for taking care of my requests.

You explanation regarding the dependenvies is fully convincing since nltk is an extra dependency of lunr. Now it all makes sense. :-)
Eventually if you add the dependency to python-lunr it will be picked directly from python-lunr dependency.
In any case this is an academic discussion since you are the maintainer of both packages and thus it is you call where to place the dependency.

Now the revision:

The license is correct and the spec file follows all the Fedora guidelines.

Running fedora-review shows three warnings:

1) The license is in index.html that is not marked as %license.
2) The package name already exists in Fedora.
3) Large documentation must go in a -doc subpackage.

They are false positives:
1) is funny and bogus. The license is already included so it does not make sense to add index.html;
2) sure enough, after all this is a re-review;
3) those files need to be there and there is already a -doc subpackage.

So the package is approved.

FEDORA-2020-505423242a has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2020-505423242a

FEDORA-2020-c75b5fcaaf has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-c75b5fcaaf

FEDORA-2020-505423242a has been pushed to the Fedora 31 testing repository.
In short time you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2020-505423242a \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-505423242a

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2020-c75b5fcaaf has been pushed to the Fedora 32 testing repository.
In short time you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2020-c75b5fcaaf \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-c75b5fcaaf

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2020-c75b5fcaaf has been pushed to the Fedora 32 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2020-505423242a has been pushed to the Fedora 31 stable repository.
If problem still persists, please make note of it in this bug report.