Handle commit hash versions #200
Conversation
Prevents a warning message about test/moduleChanges.js having no tests.
This change add support for commit hash versions on both PR diff and title. They will be merged independently of the target option since the hash doesn't have that information.
This will prevent one issue where the PR was not merged and failed silently when there wasn't a target defined
src/action.js
Outdated
|
|
||
| const packageName = getPackageName(pullRequest.head.ref) | ||
|
|
||
| return { [packageName]: { delete: semverCoerce(oldVersion).raw, insert: semverCoerce(newVersion).raw } } | ||
| if (semverCoerce(oldVersion) && semverCoerce(newVersion)) { |
There was a problem hiding this comment.
there's a better way to write this whole section using conditional chaining, let's leverage that
There was a problem hiding this comment.
I guess it could be improved, but not by just doing something like this (as it's what I'm assuming you mean:
return {
[packageName]: {
delete: semverCoerce(oldVersion)?.raw ?? oldVersion,
insert: semverCoerce(newVersion)?.raw ?? newVersion
}
}I just noticed that semver.coerce sometimes will be a bit too loose when handling some inputs which caused some of the tests to ffail, for instance it will parse cc221b3 as 221.0.0 and such. I'll use a ternary based on semver.valid which will reduce the code repetition.
I'm just fixing the code coverage that was reduced after this change.
src/util.js
Outdated
| @@ -54,3 +54,7 @@ exports.getPackageName = (branchName) => { | |||
|
|
|||
| return packageName | |||
| } | |||
|
|
|||
| exports.isCommitHash = function(version) { | |||
| return /^[\w]{7}$/.test(version) | |||
There was a problem hiding this comment.
this doesn't feel very robust. how do we know it will be always 7 chars? we don't!
There was a problem hiding this comment.
I've updated it to this. Do you think that's alright?
|
let's merge and cut a patch release please @guilhermelimak |
Also add a new test case for a full SHA1 hash.
I don't have write permissions to this repo |
This change re-add support for commit hash versions on both PR diff and title (which I believe were removed after the PR diff changes). They will be merged independently of the target option since the hash doesn't have that information.
Checklist
npm run testandnpm run benchmarkand the Code of conduct
Closes #199
Closes nearform-actions/github-action-notify-release#189