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

add option to eol just the package docs #1695

Draft
wants to merge 1 commit into
base: devel
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/docsite/rst/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@
html_context = {
'display_github': 'True',
'show_sphinx': False,
'is_eol': False,
# Change First false to true for core EOL, 2nd false to true for package EOL
'is_eol': False if tags.has('core') else False,
Comment on lines +230 to +231
Copy link
Member

Choose a reason for hiding this comment

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

Why can't you just add an additional tag and set it on the calling side? This would give you more control/flexibility with less hardcoding.

Suggested change
# Change First false to true for core EOL, 2nd false to true for package EOL
'is_eol': False if tags.has('core') else False,
'is_eol': tags.has('is_eol'),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @webknjaz ! I'm trying to understand how this would work. So once a repo goes EOL, it's always published as eol. So instead of having one place here in the conf.py to set the eol flag, I'd have to set it in multiple places around https://github.com/ansible/ansible-documentation/blob/devel/docs/docsite/Makefile#L104 (the calling side) plus the nox commands?

Copy link
Contributor

Choose a reason for hiding this comment

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

If my understanding is correct, once the suggestion from @webknjaz is merged then you would need to update two places in the Makefile to EOL the package docs:

htmldocs: ansible_structure generate_rst

and

singlehtmldocs: ansible_structure generate_rst

For example, currently that the html target is this:

CPUS=$(CPUS) $(MAKE) -f Makefile.sphinx 'DOCS_VARIANTS=-t ansible' html

To EOL the package docs, you would update that line to something like this:

CPUS=$(CPUS) $(MAKE) -f Makefile.sphinx 'DOCS_VARIANTS=-t ansible -t is_eol' html

I don't think we would need to touch the nox commands at this stage. They only call the Makefile.

Ofc you'd need to then make other updates to the Makefile separately to EOL the core docs when the time comes. So maybe it's a little error-prone to do that. I do like the idea of having a central config where you can switch eol on or off.

Maybe it would be possible to add some logic to the version helper somehow: https://github.com/ansible/ansible-documentation/blob/devel/docs/docsite/version_helper.py

Having a list of EOL versions somewhere could work. From the maintainer perspective, adding a version to an EOL list at the appropriate time seems like a natural, intuitive approach. Just a thought.

Copy link
Member

Choose a reason for hiding this comment

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

@oraNod that config should probably live where make targets are being called, externally to the Makefile.

Copy link
Member

Choose a reason for hiding this comment

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

@samccann in general, the idea is that this is not to be hardcoded in the docs generator but parametrized. And those params would govern how this flag is to be computed.

'github_user': 'ansible',
'github_repo': 'ansible-documentation',
'github_version': 'devel',
Expand Down