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

Automate more of the release process and document it (including rationale) #860

Closed
bpringe opened this issue Dec 5, 2020 · 19 comments
Closed

Comments

@bpringe
Copy link
Member

bpringe commented Dec 5, 2020

I think we can improve the CI/CD to publish from the master branch when a commit is tagged. I just realized that after the last release I must have forgotten to merge dev into master, but now dev is newer and another release needs to be made before a merge.

I think we can prevent this issue and ensure that what's on master is always the latest release by having a similar process as now, but instead of making the deploy happen from dev when a tag is pushed, we can make it happen on master.

I'm not sure this is wise though. What do you think, @PEZ?

@bpringe bpringe added the CI/CD label Dec 5, 2020
@bpringe bpringe changed the title Change CI/CD to publish release from master branch Automate more of the release process Jan 25, 2021
@bpringe
Copy link
Member Author

bpringe commented Jan 25, 2021

Adding some more thoughts here. I think more of the current process can be automated, making releases easier and less error-prone. I'll provide a framework for how I'd like the process to work, and we can discuss what needs to be changed about it.

  1. We start developing off of published instead of dev. @PEZ I think you've mentioned there were issues with this, so I know maybe this will need to stay as is, but we can discuss here.

  2. When a merge happens to published, the CI kicks off and does the following (making it kick off from published can alleviate an issue where we forget to merge dev to published after releasing, or we forget to commit/push, which has happened to me):

    a. Bumps the version number (alleviating the manual bump)

    b. Updates the changelog, moving any items under the "Unreleased" header to a release header, formatted as those currently are. (This would help alleviate an issue we experienced, where we started formatting those slightly differently without releasing and this caused the items to not be placed the Github release description.)

    c. Creates the Github release from the version number (I haven't looked at how exactly this is done, but if we can remove the manual tag requirement, that would be great.)

    d. Runs the rest of the existing pipeline steps, but without the need to approve the release. (Maybe this is not a good idea, but so far I don't see how removing it would be bad - I also don't know/remember the reason it was added, though.)

@PEZ
Copy link
Collaborator

PEZ commented Jan 26, 2021

The issue with dev is that we want to be able to pull in several changes in a release. If a user visits the repo I want him to see the README info for what is released. So I think that accumulating reviewed changes on dev and testing them integrated there before publishing makes perfect sense. But we should of course try to avoid manual steps that can bring us into trouble like that in the OP.

I like to think about these things in a ”documentation first” perspective. It is enough to look at the current instructions for how to publish a release to see that it is a bit error prone. What if the instructions looked like so:

  1. Make sure that dev represents the release you want to publish
    1. The CHANGELOG is correct, with the right content under Unreleased
    2. The README has the right information
  2. Run npm run publish_to_marketplace
  3. Approve the publishing in the CI pipeline
  4. When the marketplace shows the update, inform #calva users

I'm not sure how easy it is to achieve, but I don't see any obvious blockers. We need to figure out how to commit changes from CI, but I've done that in other pipelines (even if on Gitlab). I imagine that the npm script will start by pushing a tag, and probably the rest can happen in CI.

@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

So I think that accumulating reviewed changes on dev and testing them integrated there before publishing makes perfect sense.

So, if accumulating changes is the goal, then this makes sense, but even if we didn't have a dev branched and we developed features off of published, all changes would be tested integrated together still because we'd be merging in published and testing the changes together, as we do now, basically, with dev. Maybe I misunderstand something though.

Would the npm task to publish also merge dev to published? I don't know if you meant that, but that is something that I'd like to automate more so it's not something that can be forgotten after release.

Is it necessary to have the approve button? I assume this is to ensure that not just anyone can publish, so I suppose that makes sense, but this also works I think by restricting who can merge to published, and making the merge to published kick off the release process. Then there's no need to approve because only authorized devs can merge to published (eliminating one more step).

@PEZ
Copy link
Collaborator

PEZ commented Jan 26, 2021

I mean since I both want the default README people find to reflect what is published and want to accumulate changes, published can't be used. While we are accumulating it would show a README (and have code) that is not published.

Would the npm task to publish also merge dev to published? I don't know if you meant that, but that is something that I'd like to automate more so it's not something that can be forgotten after release.

Yes, we're om the same page here. I meant that those proposed instructions of mine would be all. When those steps are done, we are done and nothing can slip through because we forget it.

About the approve step, it is not about preventing someone to publish. All who have push access to this repo can and should be able to release, I think. (Even if it is only you and I who dare. 😄). The step is there because I sometime need to chill a bit and have a chance to change my mind. I sometimes inspect the VSIX there and sometimes also try it out.

@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

While we are accumulating it would show a README (and have code) that is not published.

This is not what I meant, sorry if I was unclear. What I meant was that if a merge/push to published causes a release, then the readme and code there will always be what is released. I still get that this is about accumulating changes more (if using a dev branch) vs not. So this is really about faster, more incremental (but still well tested) releases, vs a more long-term (but not much longer), less granular approach as we do now. I personally would like to try the former, but ultimately it's fine as is if that's important to you.

The step is there because I sometime need to chill a bit and have a chance to change my mind. I sometimes inspect the VSIX there and sometimes also try it out.

Fair enough 😄

@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

I personally would like to try the former, but ultimately it's fine as is if that's important to you.

Also, to be fair, maybe I'm trying to lump more than one concern into this issue. Maybe a step toward more automation with the current approach is best.

@PEZ
Copy link
Collaborator

PEZ commented Jan 26, 2021

What I meant was that if a merge/push to published causes a release, then the readme and code there will always be what is released

And what I mean is that I do not want a merge of a PR to cause a release. Because then we cannot accumulate changes and test them together before releasing.

This is called Real Options strategy. If we merge PR's to dev we have the option to accumulate the change plus the option to release it. If we automatically release, we lose one of the options.

@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

I see that having the dev branch gives us the option to accumulate changes before release, so I agree with keeping it, but even if we didn't have it, all new changes would still be tested with other latest changes since we'd be merging them in from published, just as we do with dev now. So I don't see any difference from a testing/integration standpoint. (I could be wrong, of course. 😄) Like I said though, I do agree with keeping dev since it gives us the option to accumulate.

Let's forget about PRs of changes going to published causing a release, as you said you don't want that, but let's consider a merging of dev into published causing a release, which eliminates a CD pipeline step.

So going off your proposed steps above:

  1. Make sure that dev represents the release you want to publish
    1. The CHANGELOG is correct, with the right content under Unreleased
    2. The README has the right information
  2. Merge dev into published and push, starting the release pipeline (we have control over this, we know it will cause a release, and we have the approve button to add a check just in case we decide against it)
  3. Approve the publishing in the CI pipeline
  4. When the marketplace shows the update, inform #calva users

Keep in mind that the bumping of the version and the publishing of the docs would be done by the CI pipeline. (I don't know the details of the version bump + commit but I assume it's possible.) This also ensures that published is basically always the source of released truth, since the release + docs publish only happens on a merge/push.

@PEZ
Copy link
Collaborator

PEZ commented Jan 26, 2021

I have, at least twice, happened to merge PRs that are pointed at master/published. The approve step would stop it, but anyway...

Also, that updating some docs on published would cause a release seems like it closes options for us, doesn't it? I'm super tired right now, so my brain doesn't really work.

@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

Those are very good points! Ok... no merge release trigger. We'll keep the npm script and just automate the existing process better. 👍

@PEZ
Copy link
Collaborator

PEZ commented Jan 26, 2021

Not sure what pipeline step is saved by the merging vs the npm script, but we could maybe push towards a release branch, or something, and that would start the release pipeline. Or, I'll just shut up now and return with a rested brain. 😀

@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

I was referring to if we add a pipeline step that merges dev to published after successful release.

@PEZ
Copy link
Collaborator

PEZ commented Jan 26, 2021

Ah, so then pushing to a third branch would risk add pipeline steps instead. 😂

@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

I think with the current improvements we want, adding a pipeline step to merge dev to published is a good idea. Not sure about if we were to use a release branch.

@bpringe bpringe changed the title Automate more of the release process Automate more of the release process and document it (including rationale behind the method) Jan 26, 2021
@bpringe
Copy link
Member Author

bpringe commented Jan 26, 2021

Added a note in the title to document the release process after we improve it. Was thinking if we include points from this discussion then we're less likely to have to remember or repeat ourselves in the future.

@PEZ
Copy link
Collaborator

PEZ commented Jan 30, 2021

We should try to get the docs updated and published as part of a release too, right?

@bpringe
Copy link
Member Author

bpringe commented Jan 30, 2021

Yeah, I'm planning to include that with these changes.

@bpringe
Copy link
Member Author

bpringe commented Feb 1, 2021

Taking a shot at revising these steps before working on them:

  1. Checkout dev
  2. Run npm run publish (below sub-steps are performed by this script)
    a. Update changelog, moving items under Unreleased header to a new header formatted the same as current release headers
    b. Commit
    c. Tag commit with version from package.json
    d. Push to dev using --follow-tags
  3. CD runs as it does now, with the below steps added to it
  4. After release is published, merge dev onto published, then push. *
  5. Run npm run deploy-docs *
  6. Checkout dev and run npm run bump-version *
  7. Commit with a relevant message and push. *

* = new steps added to CD

So, with the above, when we're ready to release, we would, from dev:

  1. Run npm run publish
  2. Wait until the pipeline gets to the approval step in CircleCI, click Approve
  3. Wait until the pipeline finishes, then test the new marketplace version
  4. If all is well, we're done. dev has already been automatically merged to published and the version was bumped in dev. If the marketplace version doesn't work, same as current instructions, we test the release build and if it works, cut a new release.

@PEZ
Copy link
Collaborator

PEZ commented Feb 1, 2021

I like it!

@bpringe bpringe changed the title Automate more of the release process and document it (including rationale behind the method) Automate more of the release process and document it (including rationale) Feb 5, 2021
@bpringe bpringe closed this as completed in 6620668 Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants