Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ext_autodoc: support RST anonymous link #542

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

tlvu
Copy link
Collaborator

@tlvu tlvu commented Jul 28, 2020

Overview

Sphinx "Duplicate explicit target name" warning is problematic when
warning are turned into errors (sphinx-build -W).

And we would want warnings to turn into error because autodoc import
failure are treated as warnings only, resulting into silent
documentation build failure if those warnings do not fail the build.

Related Issue / Discussion

See PR bird-house/cookiecutter-birdhouse#96 with the silent doc build error.

See commit bird-house/flyingpigeon@c97ebca and bird-house/emu@cfcaedf for actual real usage of this change.

Additional Information

Contribution Agreement

(as per https://github.com/geopython/pywps/blob/master/CONTRIBUTING.rst#contributions-and-licensing)

  • [x ] I'd like to contribute [feature X|bugfix Y|docs|something else] to PyWPS. I confirm that my contributions to PyWPS will be compatible with the PyWPS license guidelines at the time of contribution.
  • I have already previously agreed to the PyWPS Contributions and Licensing Guidelines

Sphinx "Duplicate explicit target name" warning is problematic when
warning are turned into errors (`sphinx-build -W`).

And we would want warnings to turn into error because autodoc import
failure are treated as warnings only, resulting into silent
documentation build failure if those warnings do not fail the build.

See PR bird-house/cookiecutter-birdhouse#96 with
the silent doc build error.
@tlvu
Copy link
Collaborator Author

tlvu commented Jul 28, 2020

This change is used in bird-house/flyingpigeon#336

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 75.589% when pulling 2a55b6e on Ouranosinc:autodoc-anonymous-link-support into 532a349 on geopython:pywps-4.2.

@tlvu
Copy link
Collaborator Author

tlvu commented Jul 28, 2020

Ping @cehbrecht @huard

Copy link
Collaborator

@cehbrecht cehbrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments inline ...

@@ -8,6 +8,21 @@
from pywps.app.Common import Metadata


class MetadataUrl(Metadata):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add the anonymous option to the Metadata class itself instead of sub-classing it. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial thought until I saw how the Metadata class is being used. It's being used to generate the xml response, it can be serialized to and deserialized from json format. So bigger impact and not required for what I needed to solve.

So I am unsure if adding the new anonymous option there is appropriate.

But it's my first pywps commit so I will follow you guidance. Let me know if adding it to the Metadata is the right thing to do.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomkralidis What is your opinion?

@tlvu
Copy link
Collaborator Author

tlvu commented Jul 29, 2020

Note to the person merging this PR, please do not use squash merge.

Squash merge will not merge the original commit and I have referenced the original commit in https://github.com/bird-house/flyingpigeon/blob/baf953244dee562b04b966dfb309987ed0ab588d/Makefile#L57 and in https://github.com/bird-house/emu/blob/3620ae99c5c5edb735e8b53e3bd0c1f0c650dd23/Makefile#L75

Alternatively please give me write access to https://github.com/geopython/pywps and I'll merge this PR myself, after approval or other required change of course.

@cehbrecht
Copy link
Collaborator

Note to the person merging this PR, please do not use squash merge.

Squash merge will not merge the original commit and I have referenced the original commit in https://github.com/bird-house/flyingpigeon/blob/baf953244dee562b04b966dfb309987ed0ab588d/Makefile#L57 and in https://github.com/bird-house/emu/blob/3620ae99c5c5edb735e8b53e3bd0c1f0c650dd23/Makefile#L75

Alternatively please give me write access to https://github.com/geopython/pywps and I'll merge this PR myself, after approval or other required change of course.

@tlvu I have given you write access as a contributor. Lets wait for feedback by @tomkralidis and @jachym.

@cehbrecht
Copy link
Collaborator

@tlvu feel free to merge :) I can make a patch release afterwards.

@tlvu tlvu merged commit 685c49f into geopython:pywps-4.2 Jul 30, 2020
@tlvu tlvu deleted the autodoc-anonymous-link-support branch July 30, 2020 13:41
tlvu added a commit to bird-house/emu that referenced this pull request Jul 30, 2020
…ust-doc-build

Refresh cookiecutter for more robust doc build.

This PR applies the cookiecutter PR bird-house/cookiecutter-birdhouse#96 to ensure no silent ReadTheDocs build failure and for Travis-CI to also catch ReadTheDocs build failure before PR is merged.

Successful RtD generation: https://emu.readthedocs.io/en/test-rtd-build/ and matching RtD build logs: https://readthedocs.org/api/v2/build/11555381.txt

Changes:

* Turn RtD warnings to build failure so they do not fail silently.

* Change Travis-CI build config to also build Epub and Latex doc format to catch RtD failure on Travis-CI before PR is merged.

* Various changes to remove all warnings in doc build since warnings will now fail the doc build.

  * For doc build only, to work-around warnings, use a new PYWPS release to support RST anonymous links, currently installing PYWPS from source (with commit hash locked so it is reproducible) since no release yet (PR geopython/pywps#542)

  * Anaconda build badge do not exist anymore so it had to be removed.

Unrelated changes part of this PR (sorry !):

* Other cookiecutter changes that are unapplied previously are also part of this PR.  You might want to look at each commit to avoid the noises.
@cehbrecht cehbrecht mentioned this pull request Aug 4, 2020
1 task
@tlvu tlvu mentioned this pull request Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants