-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Commit PR notes to draftlogs and add scripts to provide draft release notes (CHANGELOG.md) and empty draftlogs #5780
Conversation
I like it! |
all.Changed.push(message); | ||
} else if(filename.endsWith('_fix.md')) { | ||
all.Fixed.push(message); | ||
} |
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.
Else throw an error, logging the filename as having been ignored.
Might want to also loosen the endsWith
tests - to eg filename.toLowerCase().indexOf('_add') !== -1
so stuff like 5678_Added.md
will still be picked up.
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.
I think we have a test which disallows use of uppercase in filenames.
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.
We do test for uppercase, but I don't think it'll dig into the draftlogs
dir. And there's still add vs adds vs added etc. Anyway the main thing is to throw an error if we ignore the file. After adding that if you still want to be strict about names that's ok, it'll just occasionally make more work during release if we get an incorrect name.
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.
Good call. Addressed in 3a91c0e.
Looks good! Can you add notes on creating these md files to the PR template, and add notes on the new package scripts to the release procedure? |
Good call. Done in eefab9b. |
Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: Alex Johnson <[email protected]>
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.
Looks good! 💃
Resolves #5778.
Running
simply updates
CHANGELOG.md
.@plotly/plotly_js