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

FIX Handle double underscored PR titles in changelog #250

Merged

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Mar 5, 2024

Issue https://github.com/silverstripeltd/product-issues/issues/844

There was a __CLASS__ in one of the PR titles. The new markdown linter didn't like this

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

I don't want to merge a partial solution like this. There are a bunch of other scenarios where the markdown will incorrectly format what should be unformatted content in commit messages.

We should either surround the whole commit in backticks (and we'd need to check if there are already backticks in the content and double-up the wrapper backticks if neccessary) or parse the markdown and backtick anything that gets rendered as non-plain-text.

@GuySartorelli
Copy link
Member

There are also other options, such as having CI on PRs check commit messages and fail if the message has markdown that will render poorly.

Maybe open a card and we can discuss options.

@emteknetnz
Copy link
Member Author

emteknetnz commented Mar 5, 2024

A better solution is desirable, though needs to be prioritized and there's no guarantee of that. This is still better than nothing and fixes an immediate problem. We can still merge this in the meantime and replace it later with a better solution.

Note: there's no guarantee that my laptop with this patch will be used to do the RC or the stable release, someone other than myself may do it, so I think we need something merge here.

@emteknetnz
Copy link
Member Author

Have updated PR to test commit message for markdown, if it's markdown then surrond the whole message with backticks

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Seems okay.

@GuySartorelli GuySartorelli merged commit 27e20e3 into silverstripe:master Mar 5, 2024
9 checks passed
@GuySartorelli GuySartorelli deleted the pulls/master/class branch March 5, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants