Use imperative mood in changelog entry #131
Conversation
The imperative mood is generally recommended for git commit messages. Git uses it for things like "Revert ..." or "Merge ..." and dependabot uses it in the [commit message][1]. This changes the changelog helper to use the imperative mood for consistency. [1]: dangoslen@11616d3 Signed-off-by: Andrew Ross <andrross@amazon.com>
andrross
force-pushed
the
use-imperative-mood
branch
from
88bba05
to
456d982
Compare
Preview of Release Notes to be CreatedAdded
Changed
Dependencies
|
Obviously this PR would result in a bit of a jarring change in existing CHANGELOGs... |
|
@andrross not being a pedant at all. I think it could be a good idea to make this a configuration option of I do think we would need to mark this as a breaking change since it changes the behavior and I could see other teams wanting the opposite. |
I agree with all of this. It's definitely a breaking change but I like the idea of making it the new default. Configuration options do add complexity but it seems likely there will be other users who prefer the original so giving them the option will be important. There may even be other pedants who prefer |
|
Yep - I had thought about this early on (even wrote an issue for it), but decided I'd rather get something out. And while I'd like to create a more general "format string" pattern, there are some complications around finding matching lines. Using the idea of a prefix might be the easiest way to give some control, but not have to rethink several aspects of the tool (which I'm already working on a refactor or too). If you want to add the config option here I'm happy to collab on it! Or also happy to accept these changes as is and I can work adding that config before creating a |
|
@dangoslen Let me take a crack at adding this flag to this PR. If it becomes too involved I'll follow up here and we can do the two step approach you suggested. |
Signed-off-by: Andrew Ross <andrross@amazon.com>
|
Hey @andrross, the Changelog Enforcer failed. Can you take a look at the error below and correct it? Thanks! |
|
@dangoslen I made an attempt, including updating the version to 3.0. As you can see I don't really know what I'm doing here :)
|
package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "dependabot-helper", | |||
| "version": "2.2.0", | |||
| "version": "3.0.0", | |||
There was a problem hiding this comment.
I'll bump the version in a different PR. Can you put this back to 2.2.0?
The check for if the latest version is in the changelog is broken (deals with how tags get pulled across forks..) This is a good reminder I need to remove that (and a few other things) for v3.0 - 😅
src/changelog-updater.ts
Outdated
| } | ||
|
|
||
| function buildEntryLineStartRegex(entry: DependabotEntry): RegExp { | ||
| return new RegExp("- \\w+ `" + entry.package + "` from ") |
Signed-off-by: Andrew Ross <andrross@amazon.com>
Preview of Release Notes to be CreatedAdded
Changed
Dependencies
|
Signed-off-by: Andrew Ross <andrross@amazon.com>
Preview of Release Notes to be CreatedAdded
Changed
Dependencies
|
|
|
||
| ## [v1.0.0] | ||
| ### Dependencies | ||
| - Bump \`package\` from v1 to v2`) |
There was a problem hiding this comment.
Note there is some subtlety here regarding finding existing entries/duplicates. I've chosen not to change the prefix word of an existing entry if a different prefix word is provided.
|
@andrross I think this looks good! Thanks for the contribution. I do have a patch release in another PR that I will merge later tonight/tomorrow morning and then will get to merging this later this week. You PR is forcing me to do the clean-up I've been needing to do, so apologies that this won't get super fast, but I'd like to get some dependencies updated and the CI cleaned back up before doing a v3.0. Is that acceptable to you? |
Yeah, no problem at all! |
|
Hey @andrross after merging the patch PR and upgrading some deps, there looks to be a view conflicts. I don't think they are too bad though. Do you mind fixing those up? For the |
Signed-off-by: Andrew Ross <andrross@amazon.com>
Apologies in advance for being a pedant, but we in the OpenSearch project like to keep our changelog entries consistent and the dependabot changelog helper is an outlier here. Matching the style of the commit message from dependabot seems appropriate, but please feel free to let me know if you disagree :)
The imperative mood is generally recommended for git commit messages. Git uses it for things like "Revert ..." or "Merge ..." and dependabot uses it in the commit message. This changes the changelog helper to use the imperative mood for consistency.