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

GHA to create releases #170

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

rodrigozhou
Copy link
Contributor

@rodrigozhou rodrigozhou commented Jul 19, 2024

What changed?
GHA to create releases.
If a release is created, it also creates a PR in the consumers of api-go for review.

Why?
Automation.

How did you test it?

Potential risks

@rodrigozhou rodrigozhou requested review from a team as code owners July 19, 2024 06:52
@cretz
Copy link
Member

cretz commented Jul 19, 2024

Can you give some background on the savings here on starting a GH workflow vs publishing release in GH?

@rodrigozhou rodrigozhou force-pushed the rodrigozhou/gha-create-tag branch 2 times, most recently from 9f9428a to 4aa4088 Compare December 6, 2024 18:49
@rodrigozhou
Copy link
Contributor Author

@cretz Remove potentially some mistakes when releasing a new tag like forgetting to set as latest. Also, with this workflow, we can trigger it whenever the api repo has a new release so they can be in sync.

@cretz
Copy link
Member

cretz commented Dec 6, 2024

Also, with this workflow, we can trigger it whenever the api repo has a new release so they can be in sync.

To confirm, today's approach is:

  • Click release in GH for api
  • Click release in GH for api-go

But this new approach is:

  • Click run workflow in GH for api

So you save a single click, but now you have two more GH workflows to maintain and the user can still mistakenly click release in api/api-go? If we're concerned about users reading release process documentation and may not properly make one of the two releases, how can we know that they will read the documentation to know not to use the GH release feature but instead manual workflow dispatch instead? Are we saving much or just adding GH workflows to maintain?

@rodrigozhou
Copy link
Contributor Author

rodrigozhou commented Dec 6, 2024

We can further remove the ability of manually triggering the release in api-go, and only be possible from the api repo (basically change workflow_dispatch to workflow_call). Although I'd do in separate PR to make sure it's working as it is first.

Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

I am a bit concerned about the maintenance of these two workflows for such an incredibly simple thing, but if y'all are resolving to maintain them and are now in full control of the release process of this and the API repo, ok.

.github/workflows/create-tag.yml Outdated Show resolved Hide resolved
@rodrigozhou rodrigozhou force-pushed the rodrigozhou/gha-create-tag branch from 4aa4088 to 4e20b6d Compare December 10, 2024 19:15
@josh-berry josh-berry dismissed their stale review December 10, 2024 20:30

Validation added for sdk-go in another PR. Thanks Rodrigo!

.github/workflows/create-tag.yml Outdated Show resolved Hide resolved
.github/workflows/create-tag.yml Outdated Show resolved Hide resolved
CREATE_RELEASE: ${{ github.event.inputs.create_release }}
BASE_TAG: ${{ github.event.inputs.base_tag }}
run: |
if [[ -n "$(git tag -l "$TAG")" && "$(git rev-parse "$TAG")" != "$(git rev-parse HEAD)" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to be even stricter here and make sure we're not skipping versions. At that point, I'd write a Go script to do the validation though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I wonder if there will be a case in which you want to release a patch for a previous version.

Copy link
Member

Choose a reason for hiding this comment

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

I think patches are acceptable but I've never seen us issue them.

BASE_TAG: ${{ github.event.inputs.base_tag }}
run: |
gh repo set-default ${{ github.repository }}
gh release create "$TAG" --verify-tag --latest --generate-notes --notes-start-tag "$BASE_TAG"
Copy link
Member

Choose a reason for hiding this comment

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

Let's create the tag here and publish a draft. I would rather have a human in the loop to verify and edit release notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if making it a draft will re-introduce the human error of forgetting to publish the release.

Copy link
Member

Choose a reason for hiding this comment

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

No, because there won't be a tag until the release is published AFAICT.

@rodrigozhou rodrigozhou changed the title GHA to create tags/releases GHA to create a release Dec 11, 2024
@rodrigozhou rodrigozhou changed the title GHA to create a release GHA to create releases Dec 11, 2024
@rodrigozhou rodrigozhou requested a review from bergundy December 11, 2024 22:29
fetch-tags: true
submodules: true

- name: Validate input
Copy link
Member

Choose a reason for hiding this comment

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

We should also validate that the version given here matches the api submodule version. I think we should verify that there is a release or maybe a draft referencing the api submodule commit with the same version given as input to this workflow.

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.

4 participants