Generate changelog both as RST and MarkDown #576
Conversation
felixfontein
force-pushed
the
markdown
branch
from
196c7ec to
c202ea6
Compare
felixfontein
force-pushed
the
markdown
branch
from
193165d to
ed3f149
Compare
felixfontein
force-pushed
the
markdown
branch
from
9a3b9b1 to
bf911a1
Compare
felixfontein
changed the title
|
This is now fully ready for review! 🎉 I guess we have to decide on two points:
Also: do we just decide this (especially 2), or do we need to discuss this with the whole of SC (I guess I just answered this one myself, see below :) ), or even vote on it? |
|
Dear @ansible-community/steering-committee, since you might have noticed that GitHub introduced a change that breaks rendering many RST files (https://github.com/orgs/community/discussions/86715), including the ansible-core and Ansible changelog, I've started implementing code to render the changelogs as MarkDown. The result for the Ansible changelog can be seen here: ansible-community/ansible-build-data#364 What do you think about the above questions? I.e., do we want the Ansible package to have the changelog both as RST and MarkDown? (I would include both: the MD version so users can see a rendered one on GitHub, and the RST version for backwards compatibility.) And which version do we want to link to in release announcements and from the porting guide? (There's also the related discussion whether we want to include the changelog in the docsite or something like that, but please let's not discuss that here and now.) |
|
I would also suggest both, mostly becuase of
But I'll refrain from saying more as requested. |
|
MD looks good to me |
|
If nobody objects I'll merge this tomorrow, and then also refresh (if necessary) and merge ansible-community/ansible-build-data#364. |
| builder.add_raw_rst("") | ||
| section = renderer.add_section("Release Summary") | ||
| section.add_text( | ||
| t.cast(str, release_summary), text_format=TextFormat.RESTRUCTURED_TEXT |
There was a problem hiding this comment.
If you annotate the declaration of release_summary with release_summary: str | None you should be able to get rid of the cast, but I guess it was there before.
There was a problem hiding this comment.
That results in mypy complaining:
src/antsibull/build_changelog.py:297: error: Incompatible types in assignment (expression has type "str | list[str] | None", variable has type "str | None") [assignment]
| "If not mentioned explicitly, the changes are reported in the combined changelog" | ||
| " below.\n" | ||
| " below.", | ||
| text_format=TextFormat.RESTRUCTURED_TEXT, |
There was a problem hiding this comment.
Why isn't this
| text_format=TextFormat.RESTRUCTURED_TEXT, | |
| text_format=text_format, |
?
There was a problem hiding this comment.
For this piece of text it doesn't really matter, but generally I either want to use something like TextFormat.PLAINTEXT (which isn't supported yet) or specify another explicit format which definitely matches (in this case, TextFormat.RESTRUCTURED_TEXT) to avoid potential problems with conflicting markup in other formats added in the future.
There was a problem hiding this comment.
Thanks for this @felixfontein I've given this a review mostly just by looking at the code and trying to understand what it's doing and it seems fine. I had some questions about pandoc but you answered those as well. I'm sure more experienced pythonistas might have nits or other comments but in general looks really good and is a cool solution for the changelog rendering issue. 🎉
|
@briantist @Andersson007 @gotmax23 @oraNod thanks a lot for your reviews and comments! |
This needs ansible-community/antsibull-changelog#139 to actually work. CI will horribly fail since I didn't adjust it to use that repo/branch :-)This now uses the latest antsibull-changelog 0.24.0 release, which includes MarkDown support.