V1.2.1 release notes #161
V1.2.1 release notes #161
Conversation
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "dependabot-pull-request-action", | |||
| "version": "1.1.1", | |||
| "version": "1.2.1", | |||
There was a problem hiding this comment.
v1.2.0 does exist, the package.json just didn't get bumped as our versioning is done via Releases.
I added a script wrapper for npm version we use on another action to make this a little smoother.
There was a problem hiding this comment.
Oops that was my bad. Nice catch!
There was a problem hiding this comment.
Yeah, I only caught it as I'd left updater-action at v0.0.0 through about six releases 馃槵
brrygrdn
force-pushed
the
v1.2.1-release-notes
branch
from
686a140
to
cf742d5
Compare
|
Updated to include #150 which I missed in yesterday's pass. |
| case "${o}" in | ||
| p) | ||
| patch_level=${OPTARG} | ||
| (( patch_level == 'major' || patch_level == 'minor' || patch_level == 'patch')) |
There was a problem hiding this comment.
I wasn't familiar with the double parentheses construct before this 馃槃
Could you explain to me what effect this has on the script? It look like a check to see whether the patch_level is supported, but I'm having a hard time reasoning about the impact of the check on the script.
There was a problem hiding this comment.
This is really just a defensive catch on avoiding passing a garbage argument into NPM. It also acts as a filter on arguments we don't use/permit, like prepatch.
It does mean we also prevent you actually specifying the version number to use by hand, but I consider that kind of YAGNI as in that scenario you are kind of off-script on how we work so the rest of the automated steps maybe don't help.
| new_version=$(npm version "${patch_level}" --no-git-tag-version) | ||
| git checkout -b "${new_version}"-release-notes | ||
| git add package.json package-lock.json | ||
| git commit -m "${new_version}" |
There was a problem hiding this comment.
usually we have commit as a separate step when bumping versions to double check for errors.
I notice you don't push the branch here, but maybe we should keep the existing no-commit behavior?
There was a problem hiding this comment.
Yeah, that's a good point. My reasoning for going ahead with the commit here is that we're essentially just augmenting npm version patch which goes the commit and adds a tag, where we'd prefer a release branch for review first.
Since the package changes are explicitly checked in without anything else added, this feels like a safe assumption where little can go wrong. In dependabot-core, we hold the actual checkin as there's always the chance the Changelog script we wrote ourselves misfires for example 馃
| fi | ||
|
|
||
| new_version=$(npm version "${patch_level}" --no-git-tag-version) | ||
| git checkout -b "${new_version}"-release-notes |
There was a problem hiding this comment.
should we first checkout/fetch main?
There was a problem hiding this comment.
That's a good question, I went back and forth on that.
I decided to punt on it on the assuming that the user could be releasing from a release/vX branch in future and if something went wrong with the checkout/fetch we'd have to be careful to catch it which felt like scope creep.
Highlights:
fetch-metadataworkflows can be retried, thanks @mwaddell!What's Changed
dependency-typevalues by @mwaddell in Updated README to list supporteddependency-typevalues聽#145Full Changelog: v1.2.0...v1.2.1