Skip to content
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

ci: automate release #1269

Merged
merged 7 commits into from
Dec 5, 2024
Merged

ci: automate release #1269

merged 7 commits into from
Dec 5, 2024

Conversation

hanbyul-here
Copy link
Collaborator

@hanbyul-here hanbyul-here commented Nov 21, 2024

Close #1236

Description of Changes

Add packages, and configuration to make automated release work
Add an action to release a package every Monday
Add an action to check PR title to follow conventional commit pattern

Notes & Questions About Changes

If you would like to peek at how our release will look like with automation, I experimented it in this repo. Heads-up that there are many empty releases because I was experimenting with scheduling (cron) : https://github.com/hanbyul-here/release-test/releases/

Heads-up that this doesn't do scheduled releases yet. It will be not a big lift to add it, but I wanted us to trigger the action through GitHub Action page first and then kick the scheduled releases out. Currentlh the only way to trigger this action is through UI from action tab.

Screenshot 2024-12-02 at 10 55 45 PM

Conventional commit format

For conventional commits, we have a couple of options for adoption:

  1. Require PR title to follow the conventional commit format.
  2. Require every commit message to follow the conventional commit format.

I think either approach can work as long as we maintain consistency. Personally, I find option 2 a bit challenging since features or fixes often involve multiple commits, making conventional commit prefixes redundant information.

Currently, this PR only enforces that the PR title follows the conventional commit format. However, this requires some changes to the repository setup. Specifically:

  1. We need to modify the merge commit message to use the PR title instead of GitHub's default automated message (Merged...). - We can do this through GitHub repo settings.
  2. Optionally, we could set "squash and merge" as the default merge method. This would make sure that each PR results in a single commit on the main branch, creating a cleaner commit history. (do we really need all those "WIP" commits on the main branch?)
    That said, I’m slightly hesitant about squash-and-merge because it makes it harder to trace individual commits, even though we can still use reflog to recover them if necessary.

Using conventional commit as a productivity tool

The action that I am using actually offers a lot of functionality, such as ticket number validation and labeling. I adopted the most minimum possible settings, but maybe we can take more advantage of it? @aboydnw - This is the package I am using and there are detailed writings about what it can do on its read me page: https://github.com/ytanikin/PRConventionalCommits would you find anything useful ?

Validation / Testing

I'd like to go over this PR together. Some items to check together

  • How deploy key is tied to an individual (If I need to offboard from VEDA, deploy key would need to be refreshed)
  • How to trigger release manually
  • Change GitHub PR default setting (to use the title of PR instead of creating an additional message Merged...)

Copy link

netlify bot commented Nov 21, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit e3695fd
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/67521ba6d17be90007cd3be9
😎 Deploy Preview https://deploy-preview-1269--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aboydnw
Copy link
Contributor

aboydnw commented Nov 21, 2024

@aboydnw - This is the package I am using and there are detailed writings about what it can do on its read me page: https://github.com/ytanikin/PRConventionalCommits would you find anything useful ?

I see task labeling and ticket number validation as optional features. Personally, I don't think we need either of these at this point. Just my opinion, though, feel free to implement if you disagree.

Task Labeling:

  • The only issue type label I like to use is "Bug" and that's really just for outward visibility and tracking. I think the differences between all the other issue types (feature, ci/cd, task, improvement, etc) are relatively small and don't really change anything about how we solve, communicate, or prioritize them. For example, if we were really interested in minimizing new feature requests for a while, or clearing out a bunch of tech debt tasks, then maybe this label would be more valuable. At the moment, though, I don't think we use this sort of thing in our workflow to warrant using this feature.

Ticket Number Validation:

  • I assume this is to make sure everything we do has a ticket associated with it. I think if we were really tight about story points and understanding velocity. Or if we were somehow compensated based on number of tickets we completed, then maybe something like this would be more valuable. Right now, I think this would only add an admin layer that we probably don't need. I think we're pretty good about only opening PRs that are related to tickets, we don't have people going rogue and doing work unrelated to the sprint goals. And when we do, it's usually for a really good reason and I wouldn't want to slow that down.

@AliceR
Copy link
Member

AliceR commented Nov 26, 2024

Thank you for your efforts to improve our release process @hanbyul-here !

I personally agree that prefixing every commit might be excessive, especially when writing smaller feature commits. I’d lean towards the approach of requiring the PR title to follow the conventional commit format (1. Require PR title to follow the conventional commit format).

Regarding squashing, I think it’s best to clean up the WIP/fix/comment commits locally before opening a PR or marking it as ready for review. I usually handle this with an interactive rebase, allowing me to merge, rewrite, or refine commit messages and descriptions as needed. That way, we can keep the history clean but still complete in the main branch, which can sometimes be useful for tracing back a thought or decision.


By the way, should the task type for this PR be "ci"? I see "feat" as something that typically involves user-facing changes, whereas this is only about the CI pipeline.

@hanbyul-here hanbyul-here changed the title feat: automate release ci: automate release Nov 28, 2024
@hanbyul-here hanbyul-here marked this pull request as ready for review November 28, 2024 05:12
with:
fetch-depth: 0
# @TODO: Add deploy key pair
ssh-key: ${{ secrets.DEPLOY_KEY }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that this was used to bypass the branch protection rule. But I would like to replace it to use GitHub App in the future. (Specifically, when this actions starts triggering the instance build.)

@sandrahoang686
Copy link
Collaborator

sandrahoang686 commented Dec 4, 2024

Hey Hanbyul, great stuff! I agree with the sentiment of having the PR title following format instead of individual commits. I dont think we have to do the squash and merge though, it'd be nice to preserve history of each.
lgtm 👍🏼

yarn.lock Show resolved Hide resolved
- name: Conventional Commit Validation
uses: ytanikin/[email protected]
with:
task_types: '["feat","fix", "docs", "test", "ci", "refactor", "chore", "revert"]'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ : Would general updates like to wording count as a chore or fix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends on what general updates do - if it is a fix for something I would prefix it withfix:. This cheatsheet might be helpful: https://gist.github.com/qoomon/5dfcdf8eec66a051ecd85625518cfd13#types

Copy link
Collaborator

@dzole0311 dzole0311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great overview and work @hanbyul-here

I also prefer keeping separate commits instead of squash-and-merge, especially if the commit message quality is good, I find it helpful for tracking changes when debugging issues.

# @TODO: byweekly check
# Run action at 16:15 PM on Monday (UTC)
# schedule:
# - cron: '15 16 * * 1'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note that before uncommenting this we should adjust the cron job as well to actually run biweekly ('15 16 * * 1' seems like its going to run every Monday at 16:15 PM).

Copy link
Collaborator Author

@hanbyul-here hanbyul-here Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah, I left it as a @TODO but it might have not been clear, (hopefully) made a more clear comment.

.github/workflows/release.yml Show resolved Hide resolved
@hanbyul-here
Copy link
Collaborator Author

I will merge this so we can see if we can use this action for the next week's release.
I also enabled Squash and Merge option - I just wanted to enable it and it seems that Squash and merge becomes the default when it is enabled - if you see any problem from this set-up, please let me know!

@hanbyul-here hanbyul-here merged commit 94fe328 into main Dec 5, 2024
10 checks passed
@hanbyul-here hanbyul-here deleted the 1236-release-ci branch December 5, 2024 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI/CD for releasing new version and updating instances
5 participants