-
Notifications
You must be signed in to change notification settings - Fork 8
Updates to DBX Ruby release process #86
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
base: main
Are you sure you want to change the base?
Conversation
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.
zizmor found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
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.
Changes LGTM in general - just some minor feedback.
steps: | ||
- name: "Check PR Eligibility" | ||
id: check_pr | ||
uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea |
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.
TIL this exists - interesting!
env: | ||
RELEASE_NOTES: ${{ inputs.release_message }} | ||
run: | | ||
cat <<'__RELEASE_NOTES_IKDJAIELD__' > release_notes.txt |
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.
Cat on keyboard or intentional identifier that's guaranteed to not create conflicts?
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.
Intentional; I've added a comment to justify it.
using: composite | ||
steps: | ||
- name: Check out the repository | ||
uses: mongodb-labs/drivers-github-tools/secure-checkout@58501b85eae697e451b5d1d7dba53f69f65d1909 |
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.
Here and in other places: it might be helpful to add a comment indicating what this is pointing to, either a tag or a specific commit that introduces certain functionality.
As an aside for @blink1073: I'd like to figure out a solution for using actions from this repository in other actions. Currently, if I were to introduce two actions here with one using another, there is no way to reference that action in a commit hash or tag before the PR introducing the actions is merged, necessitating a second PR.
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.
Added comments. I can understand why we want to document those references, but it really feels like there has got to be a better way. A textual description for a commit SHA is a pretty brittle reference. Would a pre-processor be useful for this?
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've been using this approach and then changing back to a version tag before merging. I had tried and failed to use relative paths outside of the GitHub Action directory.
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'll hold off on merging #84 until this is approved, then we can merge both and cut the v3 tag
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 tried a few different approaches, but all resulted in either an error or the workflow not running. Looking at https://docs.github.com/en/actions/reference/workflows-and-actions/metadata-syntax#runsstepsuses, you can use another action in your OWN repository, but not it the separate repo's repository. We're stuck with either having a single repo per action (no thanks), or using the current branch as the target for testing and then reverting back to the version tag before merging.
This PR improves the release process used by DBX Ruby products (mongo-ruby-driver, mongoid, bson-ruby, etc.). It only affects the
ruby
actions, and so should have no impact on other DBX teams.The new release process depends on a few new Rake tasks (distributed separately) that autogenerate the release notes for a new release based on pull requests and labels. This produces a new "release candidate" PR, which, when merged, will automatically trigger the release process.
The changes in this PR include a new "pr-check" action, which detects whether the current push event qualifies as a releasable commit.
It also splits the previous "publish" action into separate "build" and "publish" actions, in order to better support products (like bson-ruby) that must be built for different platforms.
The publish action has also been modified to allow the release notes to be passed in as an argument, allowing the description of the release candidate PR to be used as the release notes for the GitHub release.