Skip to content

Conversation

@DaniBodor
Copy link
Member

@DaniBodor DaniBodor commented Jun 18, 2024

This PR creates a workflow to automate the release process.

As currently implemented, this workflow can be triggered manually from the Actions tab and will create a draft release on GitHub, which takes about a minute.
The draft release can then be inspected and converted into a proper release if everything is acceptable. I prefer doing this extra human check, just to avoid anything weird happening, as any true GitHub release (but not a draft release) will trigger a PyPi release as well.

The Actions tab only allows triggering workflows from the default branch. Until then, it can be triggered from command line after installing GitHub CLI with the following command (don't forget to update the stuff between <>):

gh workflow run "Release Workflow" --ref 239_streamlined_release_dbodor -f version_level="<bumpversion_level>" -f release_branch=<branch_name>

I have created two protected test branches to check whether this action will work as intended, without messing around on main/develop. These are called test_protected_main and test_protected_develop. I have used these to create conflicts, etc, to ensure that behavior happens as intended. While testing, this filter is useful to see inspect only the current action produced here.

Specifically the workflow does the following:

  • Take inputs:
    • version update level ("patch", "minor", or "major")
    • release branch (defaults to develop)
      • this last input may be superfluous with a built-in system to select branches. I will only know that once it has been released on main.
  • Check that other actions pass to avoid releasing a problematic version.
    • I am not managing to get this to work, and do not want to invest more time in it. PR 261 contains my trial code, in case someone wants to pick this up in the future.
  • Merge release-branch into main
    • A conflict will terminate the process before release, see e.g. here
  • Bump version
    • install bumpversion, bump the version on main, push to remote
  • Create draft release
    • the draft can then manually be converted into a published release.
  • A separate job is started if the release was succesful
    • In this way, the "release" will not be listed as failed if below doesn't work (e.g. due to conflict with develop branch)
    • close PR associated to the branch, if an open one exists
    • merge main into develop and push develop (at the minimum this bumps the version on develop)
    • delete release_branch if not main or develop

Additional TODOs:

  • update releasing documentation
    • Note that some of the instructions will only be applicable once the workflow is on the main branch. Will need to double check then if it's accurate.
  • turn back on functionality to trigger PyPi releasing
    • check the PyPi release action gets triggered
      • this can only be tested once we make a proper release, but there is truly no reason why it wouldn't work
  • fix branch names in the action when we finished testing everything:
    • test_protected_main --> main
    • test_protected_develop --> develop
  • delete the above test branches from the repo

closes: #239

@DaniBodor DaniBodor force-pushed the 239_streamlined_release_dbodor branch 30 times, most recently from 3a902b8 to e9fdd1a Compare June 19, 2024 08:52
@DaniBodor DaniBodor force-pushed the 239_streamlined_release_dbodor branch from d3bbc59 to e92337b Compare June 25, 2024 14:36
@DaniBodor DaniBodor force-pushed the 239_streamlined_release_dbodor branch from e92337b to fb7d267 Compare June 25, 2024 15:00
Copy link
Contributor

@wbaccinelli wbaccinelli left a comment

Choose a reason for hiding this comment

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

Good job! I think this can be imported in the dashboard as well

Copy link
Contributor

@psomhorst psomhorst left a comment

Choose a reason for hiding this comment

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

This seems great!

I have a few comments about the process itself, and a few comments about the documentation.

I think the branching strategy is not explained well anywhere. Maybe there should be a paragraph to explain, e.g.:

## Branching strategy

`main` always contains the latest stable release version of `eitprocessing`.
`develop` contains the next release version under construction. 

When creating a new feature, one should branch from `develop`. 
When a feature is finished, a PR to pull the feature into `develop` should be 
created. After one or multiple features have been pulled into `develop`, the 
release workflow can be triggered to automatically create the new feature (minor)
release originating from `develop`. 

For bug fixes that can't wait on a feature release, one should branch from `main`. 
When the bug fix is finished, the release workflow can be triggered originating from 
the created branch. 

In principle, no releases should originate from branches other than `develop` and 
bug fix branches.

This paragraph is written assuming we don't make pre-releases, release candidates, beta's, et cetera. I think we will do this at some point (e.g. a pre-release for untested code for a specific project), but that will come later.

@DaniBodor
Copy link
Member Author

For some reason, I cannot reply to this comment directly.

Is there a risk here of merging main into release_branch not resulting in a conflict/termination, but merging release_branch into main will result in a conflict/termination?

Is it possible to, in this step, check for conflicts in the other direction?

I believe that once main is merged into the release branch without conflict, the reverse cannot have a conflict. @wbaccinelli, is this correct?

Also, does this merge of main into the release branch survive? I don't think this currently matters much, because in principle we rebase everything onto develop, which has main merged into it. However, it might lead to messy git trees unnecessarily.

It only exists in the "local" repo of the workflow itself and is never pushed back to the remote, so no.
Note that I am not rebasing any of these, which indeed will lead to an ugly git tree. However, there is a much larger chance of rebase conflicts than merge conflicts, which I wanted to avoid here, so decided to go with this.
If you prefer we can still go with rebases, but then there will be more pressure on the developer to ensure that the release branch can be rebased, without an easy way to bypass this requirement.

@psomhorst
Copy link
Contributor

For some reason, I cannot reply to this comment directly.

Is there a risk here of merging main into release_branch not resulting in a conflict/termination, but merging release_branch into main will result in a conflict/termination?
Is it possible to, in this step, check for conflicts in the other direction?

I believe that once main is merged into the release branch without conflict, the reverse cannot have a conflict. @wbaccinelli, is this correct?

I think you are correct, but I'm not 100% sure. I could not find anything on the topic on StackOverflow and ChatGPT just makes nonsensical stuff up when asked about it. So I guess it's not an issue.

Also, does this merge of main into the release branch survive? I don't think this currently matters much, because in principle we rebase everything onto develop, which has main merged into it. However, it might lead to messy git trees unnecessarily.

It only exists in the "local" repo of the workflow itself and is never pushed back to the remote, so no. Note that I am not rebasing any of these, which indeed will lead to an ugly git tree. However, there is a much larger chance of rebase conflicts than merge conflicts, which I wanted to avoid here, so decided to go with this. If you prefer we can still go with rebases, but then there will be more pressure on the developer to ensure that the release branch can be rebased, without an easy way to bypass this requirement.

I think merging here is exactly what we want. A PR into develop should be rebased before merging, but develop should be merged into main.

@psomhorst
Copy link
Contributor

This seems great!

I have a few comments about the process itself, and a few comments about the documentation.

I think the branching strategy is not explained well anywhere. Maybe there should be a paragraph to explain, e.g.:

## Branching strategy

`main` always contains the latest stable release version of `eitprocessing`.
`develop` contains the next release version under construction. 

When creating a new feature, one should branch from `develop`. 
When a feature is finished, a PR to pull the feature into `develop` should be 
created. After one or multiple features have been pulled into `develop`, the 
release workflow can be triggered to automatically create the new feature (minor)
release originating from `develop`. 

For bug fixes that can't wait on a feature release, one should branch from `main`. 
When the bug fix is finished, the release workflow can be triggered for a patch release
originating from the created branch. 

In principle, no releases should originate from branches other than `develop` and 
bug fix branches.

This paragraph is written assuming we don't make pre-releases, release candidates, beta's, et cetera. I think we will do this at some point (e.g. a pre-release for untested code for a specific project), but that will come later.

@DaniBodor
Copy link
Member Author

Also, does this merge of main into the release branch survive? I don't think this currently matters much, because in principle we rebase everything onto develop, which has main merged into it. However, it might lead to messy git trees unnecessarily.

It only exists in the "local" repo of the workflow itself and is never pushed back to the remote, so no. Note that I am not rebasing any of these, which indeed will lead to an ugly git tree. However, there is a much larger chance of rebase conflicts than merge conflicts, which I wanted to avoid here, so decided to go with this. If you prefer we can still go with rebases, but then there will be more pressure on the developer to ensure that the release branch can be rebased, without an easy way to bypass this requirement.

I think merging here is exactly what we want. A PR into develop should be rebased before merging, but develop should be merged into main.

So do I interpret correctly to leave it as is?

@DaniBodor DaniBodor requested a review from psomhorst July 5, 2024 10:50
@DaniBodor DaniBodor force-pushed the 239_streamlined_release_dbodor branch from c66a3e4 to bdd1876 Compare July 5, 2024 10:57
Copy link
Contributor

@psomhorst psomhorst left a comment

Choose a reason for hiding this comment

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

😀

One typo from my side, sorry!

@wbaccinelli
Copy link
Contributor

wbaccinelli commented Jul 5, 2024

For some reason, I cannot reply to this comment directly.

Is there a risk here of merging main into release_branch not resulting in a conflict/termination, but merging release_branch into main will result in a conflict/termination?
Is it possible to, in this step, check for conflicts in the other direction?

I believe that once main is merged into the release branch without conflict, the reverse cannot have a conflict. @wbaccinelli, is this correct?

I would also say that it sounds logical. Conflicts come from lines that have been changed by both the versions, so if there are no conflicts in one direction there should be no conflict also in the other direction. I cannot be sure though, even after having consulted Lourens.
Shouldn't we maybe directly test merging the release_branch in main instead of the other way around?

@DaniBodor
Copy link
Member Author

For some reason, I cannot reply to this comment directly.

Is there a risk here of merging main into release_branch not resulting in a conflict/termination, but merging release_branch into main will result in a conflict/termination?
Is it possible to, in this step, check for conflicts in the other direction?

I believe that once main is merged into the release branch without conflict, the reverse cannot have a conflict. @wbaccinelli, is this correct?

I would also say that it sounds logical. Conflicts come from lines that have been changed by both the versions, so if there are no conflicts in one direction there should be no conflict also in the other direction. I cannot be sure though, even after having consulted Lourens. Shouldn't we maybe directly test merging the release_branch in main instead of the other way around?

Fair. I guess the entire "Check for conflicts" section can be skipped and the --no-ff --no-commit / --continue process could be done directly in the next section of the workflow. I am fairly certain that would work, but don't feel like doing an additional round of extensive testing.
If we go that route, would it be ok to just stick with it and fix problems in (the unlikely?) case they arise, rather than do a bunch mote testing now?
@psomhorst ?

@psomhorst
Copy link
Contributor

If we go that route, would it be ok to just stick with it and fix problems in (the unlikely?) case they arise, rather than do a bunch mote testing now? @psomhorst ?

Go for it!

Co-authored-by: Peter Somhorst <127968566+psomhorst@users.noreply.github.com>
@DaniBodor DaniBodor merged commit 7b1c204 into main Jul 5, 2024
@DaniBodor DaniBodor deleted the 239_streamlined_release_dbodor branch July 5, 2024 13:02
@DaniBodor DaniBodor restored the 239_streamlined_release_dbodor branch July 5, 2024 13: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.

Streamline release process

4 participants