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

Fix #11366 #11622

Merged
merged 4 commits into from
Apr 30, 2021
Merged

Fix #11366 #11622

merged 4 commits into from
Apr 30, 2021

Conversation

embray
Copy link
Member

@embray embray commented Apr 22, 2021

This fixes #11366 by adding two things:

  • Adds astropy-dev to the intersphinx mappings. This can be used now anywhere in the Astropy docs to link to docs in the
    current development version (but particularly this is useful for linking to the Developer Guide since it now lives only in the
    development version).

  • When building the development version of the docs, astropy-dev: references are automatically resolved in the "normal" manner
    as though the astropy-dev: prefix weren't there, so it avoids using the intersphinx mapping in this case.

In the dev docs themselves it is still necessary to use some templating
in order to *not* use these links, unless I am missing a way to provide
a dummy intersphinx mapping.
@embray embray added Docs Affects-dev PRs and issues that do not impact an existing Astropy release labels Apr 22, 2021
@embray embray requested a review from pllim April 22, 2021 10:14
@embray embray added this to the v4.0.6 milestone Apr 22, 2021
@embray
Copy link
Member Author

embray commented Apr 22, 2021

Also with this change, once #11621 is done we won't need the Jinja templating at all anymore.

@pllim It seems the "Check PR change log" action complains even if the "Affects-dev" label is used.

@embray
Copy link
Member Author

embray commented Apr 22, 2021

I just realized there are some other "if is_development" sections in install.rst that were added since I last worked on this. So I'll go back to using the Jinja templating just for those sections for now, and worry about reworking those parts as part of #11621

appropriately whether or not building the dev docs:

* For dev docs it resolves to the local documentation (this includes
  when reading a local copy of the docs, e.g. when testing the docs
  during development)

* For stable docs it will use the intersphinx mapping for astropy-dev.

This obviates the need for the Jinja templating in most cases.
@pllim
Copy link
Member

pllim commented Apr 22, 2021

It seems the "Check PR change log" action complains even if the "Affects-dev" label is used.

towncrier check doesn't care about "Affects-dev" -- Maybe @Cadair can comment.

@pllim
Copy link
Member

pllim commented Apr 22, 2021

p.s. Saw your pings for reviews. It is on my todo list!

@Cadair
Copy link
Member

Cadair commented Apr 22, 2021

It seems the "Check PR change log" action complains even if the "Affects-dev" label is used.

towncrier check doesn't care about "Affects-dev" -- Maybe @Cadair can comment.

Huh, why would it?

@pllim
Copy link
Member

pllim commented Apr 26, 2021

@Cadair , because astropy-bot old-style change log check recognized it, though it was a very astropy specific thing.

@embray , if you feel strongly about adding back "Affects-dev" for the change log check, I can add that logic into https://github.com/pllim/actions-towncrier-changelog but the change will likely be lost when/if we switch back to using https://github.com/OpenAstronomy/baldrick/blob/master/baldrick/plugins/github_towncrier_changelog.py . For now, only no-changelog-entry-needed will skip the check.

@Cadair
Copy link
Member

Cadair commented Apr 27, 2021

What did it do?!

@pllim
Copy link
Member

pllim commented Apr 27, 2021

Full logic is here, but it probably won't apply as-is with new change log system: https://github.com/astropy/astropy-bot/blob/main/astropy_bot/changelog_checker.py

pllim added a commit to pllim/astropy that referenced this pull request Apr 27, 2021
@pllim pllim mentioned this pull request Apr 27, 2021
3 tasks
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Looks fine, except for a minor typo.

I am testing the "stable" build at #11652

docs/conf.py Outdated Show resolved Hide resolved
@pllim
Copy link
Member

pllim commented Apr 27, 2021

pllim added a commit to pllim/astropy that referenced this pull request Apr 27, 2021
docs/conf.py Outdated Show resolved Hide resolved
@embray
Copy link
Member Author

embray commented Apr 29, 2021

@pllim Fixed! Thanks for the confirm.

docs/index.rst Outdated Show resolved Hide resolved
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

LGTM. It's like magic. Thanks!

@pllim pllim merged commit e1c5f63 into astropy:main Apr 30, 2021
pllim added a commit to pllim/astropy that referenced this pull request Apr 30, 2021
@pllim pllim mentioned this pull request May 3, 2021
3 tasks
astrofrog pushed a commit that referenced this pull request Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects-dev PRs and issues that do not impact an existing Astropy release Docs installation no-changelog-entry-needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add intersphinx target for astropy-dev docs
3 participants