Bug 1811410 - Re-Review Request: mkdocs - Python tool to create HTML documentation from markdown sources
Summary: Re-Review Request: mkdocs - Python tool to create HTML documentation from mar...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: José Matos
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1771376 1811377 1811409
Blocks: 1815725 1815726 1815727 1815728 1815729
TreeView+ depends on / blocked
 
Reported: 2020-03-08 12:46 UTC by Robin Lee
Modified: 2020-04-05 03:03 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2020-04-05 00:15:53 UTC
Type: ---
Embargoed:
jamatos: fedora-review+


Attachments (Terms of Use)

Description Robin Lee 2020-03-08 12:46:17 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01298182-mkdocs/mkdocs.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/cheeselee/review/fedora-rawhide-x86_64/01298182-mkdocs/mkdocs-1.1-1.fc33.src.rpm
Description: MkDocs is a fast and simple way to create a website from source files written
in Markdown, and configured with a YAML configuration file, the documentation
can be hosted anywhere, even in free hosting services like Read the Docs and
GitHub Pages.
Fedora Account System Username: cheeselee

Comment 1 José Matos 2020-03-18 09:17:03 UTC
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.

Comment 2 Robin Lee 2020-03-21 07:09:16 UTC
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.

Comment 3 José Matos 2020-03-21 18:46:47 UTC
(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. :-)

Comment 4 Robin Lee 2020-03-23 11:41:39 UTC
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.

Comment 5 José Matos 2020-03-23 13:03:34 UTC
(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

Comment 6 Robin Lee 2020-03-24 11:46:54 UTC
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.

Comment 7 José Matos 2020-03-24 12:09:43 UTC
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.

Comment 8 Fedora Update System 2020-03-27 14:54:29 UTC
FEDORA-2020-505423242a has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2020-505423242a

Comment 9 Fedora Update System 2020-03-27 14:54:32 UTC
FEDORA-2020-c75b5fcaaf has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-c75b5fcaaf

Comment 10 Fedora Update System 2020-03-28 02:43:07 UTC
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.

Comment 11 Fedora Update System 2020-03-28 14:59:21 UTC
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.

Comment 12 Fedora Update System 2020-04-05 00:15:53 UTC
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.

Comment 13 Fedora Update System 2020-04-05 03:03:37 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.