-
Notifications
You must be signed in to change notification settings - Fork 17
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
Allow collections to render changelog as part of their docsite #267
Conversation
You can see the result here: https://ansible-collections.github.io/community.dns/pr/192/index.html#changelog (belongs to ansible-collections/community.dns#192). |
This is now ready for review! (I will only mark the PR as that on GitHub once the DaWGs meeting was able to talk about this.) |
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.
mostly looks good to me! been looking forward to this for a long time, I'll be able to remove my symlinks and extra docs entries in a bunch of collections, thanks for implementing!
tests/functional/baseline-no-breadcrumbs/collections/ns/col2/index.rst
Outdated
Show resolved
Hide resolved
tests/functional/baseline-no-breadcrumbs/collections/ns2/col/changelog.rst
Show resolved
Hide resolved
tests/functional/baseline-no-breadcrumbs/collections/ns2/flatcol/index.rst
Outdated
Show resolved
Hide resolved
tests/functional/baseline-no-indexes/collections/ns2/flatcol/index.rst
Outdated
Show resolved
Hide resolved
tests/functional/baseline-simplified-rst/collections/ns/col2/index.rst
Outdated
Show resolved
Hide resolved
tests/functional/baseline-simplified-rst/collections/ns2/col/changelog.rst
Show resolved
Hide resolved
tests/functional/baseline-simplified-rst/collections/ns2/flatcol/index.rst
Outdated
Show resolved
Hide resolved
tests/functional/baseline-use-html-blobs/collections/ns2/col/changelog.rst
Show resolved
Hide resolved
@@ -12,6 +12,11 @@ | |||
from antsibull_docs._pydantic_compat import v1 as p | |||
|
|||
|
|||
class ChangelogConfig(p.BaseModel): | |||
# Whether to write the changelog | |||
write_changelog: bool = 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.
Do we think we might ever default this to true? maybe in a new major version?
I guess we'd want to discuss in the forum and/or dawgs meeting.
If so, we could announce the future default value in the changelog in this PR too
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.
It should never default to true, but it could default to something like "auto" which checks whether changelogs/changelog.yaml is present and only then sets it to true. But that would have to wait for a new major release.
Also I don't think the default should switch; I'd rather have a configurable toggle for the docsite build which allows to override this for collections.
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.
Some minor comments. @felixfontein my 2c is that changelog should come at the very end of the index page, like release notes.
Thanks for this! I like the current location of where you put the changelog. In the DaWGs meeting I mentioned that the left-hand nav is quite busy in the demo page. I'm not sure if this would be the same in the full docsite or not, but figured I'd mention it. @briantist mentioned it might benefit from using 'squash heirarchy'. |
Sorry my wording may have been confusing. The preview shown here, and its "busy" left-hand navigation, is caused by using It won't look like that on the main docsite, or anywhere where the option is not explicitly used.
|
ah gotcha. thanks for clarifying! |
Co-authored-by: Don Naro <dnaro@redhat.com>
Now we have @oraNod who prefers the changelog at the bottom of the collection index page, and @samccann who likes the current location. @briantist what do you think? And are there other opinions on this? :) |
I posted on the docs channel as well in case anyone else has an opinion. |
I don't think I'm leaning too strongly in either direction. I have maybe a slight preference for its current location, but it doesn't matter that much. Mainly I wouldn't want it stuck somewhere in the middle, so near the top like it is now (just after communication), or all the way at the bottom, are decent options. |
Same here. Not a hill I'm wiling to die on. Go with either option. |
My take: As both the changelog and the plugins list can be quite long dependending on each collection: As long as the changelog is not too long, and ideally if it only shows the "big changes/deprecation warning", I would prefer the middle, so you get a glimpe of changes just by scrolling through the doc before reaching the plugins. If it might be too long, then I would move it to the bottom, to avoid disrupting the plugin scrolling (which is most likely the reason you went to the docs in the first place) |
This is only about a link to the changelog. The changelog itself is on a separate page. So don't worry about its length :) |
Oh, if it's just a link, then middle works better in my opinion. Edit: by the way, by middle I mean the Index position (3rd out of 5 in the example) |
In that particular example, the changelog is already in the 'middle'; it comes after the collection information (description, communication) and before the extra documentation (if available) and plugin/module/role index. |
Since most like the current location and there haven't been more opinions, I'm going to merge the current version. I'll wait some more days before creating a new release, so if compelling new opinions / insights come up we can still change it without breaking something that's released :-) |
Thanks everyone for your reviews and opinions! |
." This reverts commit e4cd0ec.
Fixes #31.
To do: