-
Notifications
You must be signed in to change notification settings - Fork 506
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
base: devel
Are you sure you want to change the base?
Conversation
# Change First false to true for core EOL, 2nd false to true for package EOL | ||
'is_eol': False if tags.has('core') else False, |
There was a problem hiding this comment.
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.
# 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'), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
ansible-documentation/docs/docsite/Makefile
Line 104 in 3b43d64
htmldocs: ansible_structure generate_rst |
and
ansible-documentation/docs/docsite/Makefile
Line 114 in 3b43d64
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@samccann I'd like to suggest another approach to this. What if we maintained a file in the repo that contains something like this?
I would prefer a declarative approach to this for clarity and maintainability. It could also improve how we approach branching to stay in sync with core stable versions. For the eol banner, I think we could use a sphinx role to inject the banner at build time for any version in the For core stable branches, we can periodically run a workflow that checks out the core repo, compares branches against the I'm happy to take a stab at this but wanted to put the idea out. The branch announcement would be pretty straightforward. |
e17b182
to
9df9fcd
Compare
@oraNod finally getting back to this. I sort of understand what you are thinking in your last comment, but I don't know how to implement it, so if you want to give it a go, sure. I can close this PR. The only requirement (other than make it easy pls :-) is that core and package docs EOL at very different times, and package EOL's first. |
Thanks @samccann I can try and play around with this. Actually have an idea of how it might work without touching the makefile. I've put the comments and stuff in the GH issue for this if you want to go ahead and close. |
Fixes #1490