Omit changelog header from GitHub release notes #17165
Conversation
rustbot
added
the
S-waiting-on-review
Marcono1234
changed the title
Marcono1234
force-pushed
the
github-release-changelog-header
branch
from
e930b39
to
d502ed0
Compare
xtask/src/publish.rs
Outdated
| @@ -9,6 +9,22 @@ impl flags::PublishReleaseNotes { | |||
| pub(crate) fn run(self, sh: &Shell) -> anyhow::Result<()> { | |||
| let asciidoc = sh.read_file(&self.changelog)?; | |||
| let mut markdown = notes::convert_asciidoc_to_markdown(std::io::Cursor::new(&asciidoc))?; | |||
|
|
|||
| // Remove changelog header because GitHub misinterprets the changelog number as issue reference | |||
There was a problem hiding this comment.
| // Remove changelog header because GitHub misinterprets the changelog number as issue reference | |
| // Remove changelog header because it's not going to be relevant for the GitHub release notes |
xtask/src/publish.rs
Outdated
| let changelog_header_prefix = "# Changelog #"; | ||
| let mut changelog_header_start = | ||
| markdown.find(changelog_header_prefix).expect("should contain changelog header"); | ||
| changelog_header_start = |
There was a problem hiding this comment.
This seems a little too complicated, and I'm not sure why it's needed.
My suggestion is to edit the AsciiDoc source, not the derived Markdown, since the latter is less predictable. That way, we only need to trim off the first line (until the first '\n').
We could also strip off the Commit: ... line because it's redundant, but that seems less important.
There was a problem hiding this comment.
Good point; but it seems the AsciiDoc to Markdown conversion currently expects a title, and otherwise fails with "Error: document title not found". And I did not want to remove that code.
So I have changed it now to only remove the # from the changelog number. But since this deviates a bit from what this pull request was originally about, I have marked it as Draft for now again. What do you think?
There was a problem hiding this comment.
Sorry, I didn't realize the title was needed for AsciiDoc.
I'd still prefer to strip off the line, since it's useless there.
There was a problem hiding this comment.
Maybe the changelog number is a bit useful though? For example when someone refers to it because they read it on the project website. I don't think GitHub itself numbers the releases.
In case you still think the complete title should be removed, should I then also remove the code which tries to convert this AsciiDoc title to Markdown? Or how should I handle the title being required currently?
(Also, in case it is easier for you to directly take this over, just let me know; this PR has "Allow edits by maintainers" enabled in case that is helpful.)
Resolves #16455
I wasn't sure whether one of you maintainers wanted to implement this, so I gave it a try. Feedback is appreciated! No worries if you reject this pull request (and implement the changes yourself) in case these changes here are not clean enough.
I have tested this using
test_data/input.adoc(renamed to2024-01-01.adoc) with the following command:It seems to work correctly, but would be good if you could double check.