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: revert file changes and add some checks #12873

Merged
merged 3 commits into from
May 5, 2022
Merged

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented May 4, 2022

During the release there are several files that need to be modified:

  • .release/ci.hcl: the notification channel needs to be updated to a
    channel with greater team visibility during the release.
  • version/version.go: the Version and VersionPrerelease variables
    need to be set so they match the release version.

After the release these files need to be reverted.

For GA releases the following additional changes also need to happen:

  • version/version.go: the Version variable needs to be bumped to the
    next version number.
  • GNUMakefile: the LAST_RELEASE variable needs to be set to the
    version that was just released.

Since the release process will commit file changes to the branch being
used for the release, it should never run on main, so the first step
is now to protect against that.

It also adds a validation to make the user input version is correct.

During the release there are several files that need to be modified:

  - .release/ci.hcl: the notification channel needs to be updated to a
    channel with greater team visibility during the release.
  - version/version.go: the Version and VersionPrerelease variables
    need to be set so they match the release version.

After the release these files need to be reverted.

For GA releases the following additional changes also need to happen:

  - version/version.go: the Version variable needs to be bumped to the
    next version number.
  - GNUMakefile: the LAST_RELEASE variable needs to be set to the
    version that was just released.

Since the release process will commit file changes to the branch being
used for the release, it should _never_ run on main, so the first step
is now to protect against that.

It also adds a validation to make the user input version is correct.
@lgfa29 lgfa29 added backport/1.0.x backport to 1.0.x release line backport/1.1.x backport to 1.1.x release line backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line labels May 4, 2022
@lgfa29 lgfa29 requested review from DerekStrickland, tgross, a team, shore and jeanneryan May 4, 2022 17:56
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 153 to 158
- name: Revert notification channel
if: ${{ github.event.inputs.notification-channel != '' }}
run: |
sed -i.bak -e 's|\(notification_channel * = *"\)[^"]*|\1${{ steps.notification-channel.outputs.prev-channel }}|g' .release/ci.hcl
rm -rf .release/ci.hcl.bak
git diff --color=always .release/ci.hcl
Copy link
Member

Choose a reason for hiding this comment

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

We're calling this "revert" but it's not obvious whether this literally backing out "Generate files for..." commit or making a slightly different change that could potentially drift. Could we literally reset this file with git (ex. git reset $ref -- .release/ci.hcl)?

This is particularly the case for the step below too, where we're doing two different things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah, good point. This one is direct revert, the next one not quite, so I will rename it as well. Thanks!

After looking at the different release options and steps I noticed that
automatic CHANGELOG generation is actually the exception, so it would be
better to have the default to be false.
Copy link
Contributor

@DerekStrickland DerekStrickland left a comment

Choose a reason for hiding this comment

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

Apologies in advance if I'm asking a question that doesn't make sense.

Since we have conditional logic based on branch (I think?) could we use an approach similar to what is discussed here so that we don't have to manually massage files? I'm sure you already considered something like this. I'm just trying to learn more about the context / challenge.

@lgfa29
Copy link
Contributor Author

lgfa29 commented May 5, 2022

Apologies in advance if I'm asking a question that doesn't make sense.

Since we have conditional logic based on branch (I think?) could we use an approach similar to what is discussed here so that we don't have to manually massage files? I'm sure you already considered something like this. I'm just trying to learn more about the context / challenge.

I didn't quite follow that. The conditional logic is based on the prerelease part of the version being release.

The only branch check we have is to prevent it from running from main. I could extract that first step into a separate and have it as a dependency to the main one, but each job is a separate environment in GitHub Actions, which means it would have to spin a whole new infrastructure just for a return 1 😅

Is this what you had in mind?

@lgfa29 lgfa29 merged commit d7d578b into main May 5, 2022
@lgfa29 lgfa29 deleted the ci-automate-file-changes branch May 5, 2022 22:07
lgfa29 added a commit that referenced this pull request May 6, 2022
During the release there are several files that need to be modified:

  - .release/ci.hcl: the notification channel needs to be updated to a
    channel with greater team visibility during the release.
  - version/version.go: the Version and VersionPrerelease variables
    need to be set so they match the release version.

After the release these files need to be reverted.

For GA releases the following additional changes also need to happen:

  - version/version.go: the Version variable needs to be bumped to the
    next version number.
  - GNUMakefile: the LAST_RELEASE variable needs to be set to the
    version that was just released.

Since the release process will commit file changes to the branch being
used for the release, it should _never_ run on main, so the first step
is now to protect against that.

It also adds a validation to make the user input version is correct.

After looking at the different release options and steps I noticed that
automatic CHANGELOG generation is actually the exception, so it would be
better to have the default to be false.
lgfa29 added a commit that referenced this pull request May 6, 2022
During the release there are several files that need to be modified:

  - .release/ci.hcl: the notification channel needs to be updated to a
    channel with greater team visibility during the release.
  - version/version.go: the Version and VersionPrerelease variables
    need to be set so they match the release version.

After the release these files need to be reverted.

For GA releases the following additional changes also need to happen:

  - version/version.go: the Version variable needs to be bumped to the
    next version number.
  - GNUMakefile: the LAST_RELEASE variable needs to be set to the
    version that was just released.

Since the release process will commit file changes to the branch being
used for the release, it should _never_ run on main, so the first step
is now to protect against that.

It also adds a validation to make the user input version is correct.

After looking at the different release options and steps I noticed that
automatic CHANGELOG generation is actually the exception, so it would be
better to have the default to be false.
lgfa29 added a commit that referenced this pull request May 6, 2022
During the release there are several files that need to be modified:

  - .release/ci.hcl: the notification channel needs to be updated to a
    channel with greater team visibility during the release.
  - version/version.go: the Version and VersionPrerelease variables
    need to be set so they match the release version.

After the release these files need to be reverted.

For GA releases the following additional changes also need to happen:

  - version/version.go: the Version variable needs to be bumped to the
    next version number.
  - GNUMakefile: the LAST_RELEASE variable needs to be set to the
    version that was just released.

Since the release process will commit file changes to the branch being
used for the release, it should _never_ run on main, so the first step
is now to protect against that.

It also adds a validation to make the user input version is correct.

After looking at the different release options and steps I noticed that
automatic CHANGELOG generation is actually the exception, so it would be
better to have the default to be false.
lgfa29 added a commit that referenced this pull request May 6, 2022
During the release there are several files that need to be modified:

  - .release/ci.hcl: the notification channel needs to be updated to a
    channel with greater team visibility during the release.
  - version/version.go: the Version and VersionPrerelease variables
    need to be set so they match the release version.

After the release these files need to be reverted.

For GA releases the following additional changes also need to happen:

  - version/version.go: the Version variable needs to be bumped to the
    next version number.
  - GNUMakefile: the LAST_RELEASE variable needs to be set to the
    version that was just released.

Since the release process will commit file changes to the branch being
used for the release, it should _never_ run on main, so the first step
is now to protect against that.

It also adds a validation to make the user input version is correct.

After looking at the different release options and steps I noticed that
automatic CHANGELOG generation is actually the exception, so it would be
better to have the default to be false.
@shoenig shoenig mentioned this pull request Jun 2, 2022
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.0.x backport to 1.0.x release line backport/1.1.x backport to 1.1.x release line backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants