-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2023-11-30] Manually manage tags in bumpVersion action #1612
Conversation
const addCommand = `git add ${ | ||
[PACKAGE_JSON_PATH, PACKAGE_LOCK_PATH, BUILD_GRADLE_PATH, PLIST_PATH, PLIST_PATH_TEST].join(' ') | ||
}`; | ||
const commitCommand = `git commit -m "Update version to ${newVersion}`; | ||
const tagCommand = `git tag ${newVersion} && git push --tags`; | ||
return exec(`${[addCommand, commitCommand, tagCommand].join(' && ')}`); |
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.
Not sure I have an opinion either way, but is there a reason for doing this in JS as opposed to in the action?
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.
The only reason would be to use the new version in the commit message and tag. However, one of the things on our project roadmap is having this action output the new version, so we could just output the new version from JS then do the git stuff in the workflow.
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.
Discussed offline and I think we both agreed that moving this out of JS will make our lives and code easier in the long run
.github/workflows/version.yml
Outdated
./android/app/build.gradle \ | ||
./ios/ExpensifyCash/Info.plist \ | ||
./ios/ExpensifyCashTests/Info.plist | ||
git cm -m "Update version to ${{ steps.bumpVersion.outputs.newVersion }}" |
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.
Is cm
a local shortcut you have installed locally?
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.
😅 is git commit
even a command?
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 catch! I honestly forgot that git cm
is just an alias
|
❗ Please, do not use Github auto-linking keywords such as these: close, closes, closed, fix, fixes, fixed, resolve, resolves or resolved. For more details, see the Contributing Guidelines, specifically Submit your pull request for a final review 📖. |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.2-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-11-30. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
|
Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/155197
Tests
None yet, test by merging
Tested On