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

Update DEVELOPMENT_CYCLE.md to work with [patch.crates-io] #544

Merged
merged 2 commits into from
Aug 31, 2022

Conversation

notmandatory
Copy link
Member

@notmandatory notmandatory commented Feb 16, 2022

Description

Update DEVELOPMENT_CYCLE and release instructions to make overriding dependencies possible for downstream projects with unreleased bdk versions for development and testing. Also simplifies the release process by capturing changelog information in the pull_request_template and recording release changelog information in the release tag message instead of in a CHANGELOG.md file which causes too many merge conflicts and complicates the release process.

Fixes #536
Fixes #496

Notes to the reviewers

The primary changes to our current release process are:

  1. Don't add -dev or -rc.x to unreleased bdk cargo versions because those extensions do not work with overriding dependencies.
  2. Increment the master branch version as soon as a release/MAJOR.MINOR branch is created, the next release release/MAJOR.MINOR branch version with be MAJOR.MINOR.PATCH, and the master branch development version will be MAJOR.MINOR+1.0; either version can be used with overriding dependencies.
  3. Remove the bdk version from the src/lib.rs file so that it doesn't need to be changed on every release, because it isn't needed in the rust docs for most developers and removing it will help simplify the release process.
  4. The new release process is now documented as a checklist in a new release.md github issue template.
  5. Putting changelog information in the release tag message is how the tokio project does it. After this PR is merged I will replace old tags with new ones containing changelog information and then do a new PR to remove the CHANGELOG.md file. After this PR is merged I don't think we need to update old tags, only rename the CHANGELOG.md file to CHANGELOG-OLD.md with a note to check tags for future change log info.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@notmandatory notmandatory force-pushed the doc/fix_patch branch 3 times, most recently from 42fc114 to 3100d18 Compare February 16, 2022 22:54
@notmandatory notmandatory force-pushed the doc/fix_patch branch 6 times, most recently from 010b7fb to 98ad84d Compare March 19, 2022 03:50
@notmandatory notmandatory marked this pull request as ready for review March 19, 2022 04:01
@notmandatory notmandatory self-assigned this Mar 19, 2022
Copy link
Contributor

@rajarshimaitra rajarshimaitra left a comment

Choose a reason for hiding this comment

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

Concept ACK.. Agreed that it would make quickly checking up change logs in the release message itself for downstream devs..

Just a small nit..

.github/ISSUE_TEMPLATE/release.md Outdated Show resolved Hide resolved
@danielabrozzoni
Copy link
Member

Concept ACK for this, at this point we all hate CHANGELOG.md 😄

Shouldn't we add a "## Changelog" subtitle in the PR template? Not all PRs make it into the changelog (we usually ignore doc fixes, ci fixes, minor bug fixes, refactors, etc...).

Also, this needs a rebase :)

@csralvall
Copy link
Contributor

csralvall commented Aug 10, 2022

I don't know if all the following is covered by Auto-generate release notes button,
I've never been in the role of a maintainer, but it may help anyways.

I've been doing some research regarding #543.
GitLab's final solution diverted from the initial approach using files. You can
read more about it in this issue (TLDR: the last comment) and the documentation of the
tool mentioned in the original document.
As per the comment in Gitlab Issue #1339, they decided to use commit titles as input,
and changelog files as output. That approach seems to be shared by other projects,
using different tools (there is a great variety of them). All those projects and tools
principally work through the use of special conventions in their commit
messages. Conventional commits is one of them, in the same spirit as SemVer.

It seems that tokio is using some variation of the kind.
I would like to have more details about how tokio collects all the CHANGELOG
entries in each release. Is it done automatically? Is it done manually by
maintainers?

I prefer a semiautomatic approach rather than just the maintainers editing the
CHANGELOG themselves.
The workflow that I have in mind could be roughly like this:

  1. Make changes.
  2. Add to the staging area.
  3. Commit using type, context and trailers like changelog, fix or BREAKING CHANGE in the message.
    The changelog trailer allows you to opt out of CHANGELOG entries.
  4. Push changes.
  5. Review code and commit message description and format.
  6. Accept changes.
  7. Start the Merging process and trigger CI workflow with a tool to introduce
    the new lines in CHANGELOG.

Maintainers will take part in this process during the review. When they approve
a PR, they also approve the commit messages. This doesn't require the extra
effort of the edition of files with new CHANGELOG entries, which would be in
the hands of the CI process.
Also, the enforcement of the commit message format improves the quality of the
information provided by each commit. It may be a burden for new contributors,
but it ensures quality and is a step towards a self-contained project (detached
from the current remote host, Github).

@notmandatory
Copy link
Member Author

@csralvall thanks for the info on how other projects are automating, or at least semi-automating the generation of their changelog info. I'd prefer to start with manually creating the changelog info provided by PR creators, and then later we can look into setting up some automation to help us compile this info.

Even if we go with some sort of changelog generation I'd still rather put the release summary and changelog info in the tag commits rather than a text file so that we don't ever have to merge our release branches back to the master branch. I think as long as we also keep a copy of the info in the GitHub release page it will be easy for new folks to find it. And experienced people won't have a problem finding the info in the the tag annotations.

Copy link
Member

@afilini afilini left a comment

Choose a reason for hiding this comment

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

Concept ACK, now that we are starting to have a larger volume of PRs I think we should switch to these new guidelines


On the day of a **MAJOR.MINOR.0** release:

- [ ] Add a tag to the `HEAD` commit in the `release/MAJOR.MINOR` branch.
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 here you should first add a commit to bump the version and remove the rc suffix

Copy link
Member Author

Choose a reason for hiding this comment

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

In the section starting on line 41 I bump the version for the next dev version in master. So the master branch will always be at the current dev version, this is needed so someone can patch their project and point to the master branch to use the current dev branch version.

I also added a version bump to 0.22.0 in this PR to get us ready to follow this process for the upcoming feature freeze.

.github/pull_request_template.md Show resolved Hide resolved
@afilini
Copy link
Member

afilini commented Aug 25, 2022

Wait a second, are you sure this fixes #496? If we immediately bump the version in master, then all the projects depending on a currently relased version of bdk can't switch to master, because the version has already been bumped.

For example, the current latest release is 0.21. If we merge this PR we immediately bump our master to 0.22, which makes it impossible to patch bdk from the current latest release to master

@notmandatory
Copy link
Member Author

notmandatory commented Aug 25, 2022

For example, the current latest release is 0.21. If we merge this PR we immediately bump our master to 0.22, which makes it impossible to patch bdk from the current latest release to master

Is the scenario you're thinking of for using an unreleased patch to 0.21 (scenario 3 below)? My scenarios for patching a project to use an unreleased bdk version:

  1. next MINOR (0.MINOR+1.0), untagged:
[patch.crates-io]
bdk = { git = 'https://github.com/bitcoindevkit/bdk', branch = 'master' }

[dependencies]
bdk = "0.MINOR+1.0"
  1. next MINOR (0.MINOR+1.0), tagged RC.1:
[patch.crates-io]
bdk = { git = 'https://github.com/bitcoindevkit/bdk', tag = 'v0.MINOR+1.0-RC.1' }

[dependencies]
bdk = "0.MINOR+1.0"
  1. current MINOR, next PATCH fix (0.MINOR.PATCH+1), untagged:
[patch.crates-io]
bdk = { git = 'https://github.com/bitcoindevkit/bdk', branch = 'release/0.MINOR' }

[dependencies]
bdk = "0.MINOR.PATCH+1"
  1. current MINOR, next PATCH fix (0.MINOR.PATCH+1), tagged RC.1:
[patch.crates-io]
bdk = { git = 'https://github.com/bitcoindevkit/bdk', tag = 'v0.MINOR.PATCH+1-RC.1' }

[dependencies]
bdk = "0.MINOR.PATCH+1"

@danielabrozzoni
Copy link
Member

I think the problem is:

  1. We release bdk 0.21
  2. We merge fantastic things on master
  3. Someone wants to patch bdk 0.21 with our master, to have the fantastic things
  4. The Cargo.toml version in master should be exactly 0.21, otherwise you can't patch

@notmandatory
Copy link
Member Author

I think the problem is:

1. We release bdk `0.21`

2. We merge fantastic things on master

3. Someone wants to patch bdk `0.21` with our master, to have the fantastic things

4. The `Cargo.toml` version in master should be exactly `0.21`, otherwise you can't patch

My thinking was in this case, they want some cool stuff from master then they'll need to patch to master and change the version to the one that will eventually have these things which is 0.22. If they only want some bug fix that will be in 0.21.x then they'd need to patch to the release/0.21 branch and they'll get it there.

@notmandatory
Copy link
Member Author

notmandatory commented Aug 30, 2022

As discussed on the call today, I updated the checklist for making a MINOR release to bump the master branch version just before making the release branch. This should let users [patch] to the master branch to get pre-released (and possibly breaking) changes for the current latest release.

I also decided to separated the steps for making a PATCH release into it's own issue template. And I added an extra master branch version bump when doing PATCH releases so that if someone wants to patch a dependency to master that is using a latest released patch version, it should still work.

@notmandatory notmandatory force-pushed the doc/fix_patch branch 2 times, most recently from 6e5f078 to 9483adf Compare August 30, 2022 18:49
@danielabrozzoni
Copy link
Member

I merged another PR before this one, and now the CHANGELOG is conflicting :/ Sorry!

@afilini
Copy link
Member

afilini commented Aug 31, 2022

I merged another PR before this one, and now the CHANGELOG is conflicting

How ironic

@notmandatory
Copy link
Member Author

I merged another PR before this one, and now the CHANGELOG is conflicting :/ Sorry!

Rebased :-D

Copy link
Member

@afilini afilini left a comment

Choose a reason for hiding this comment

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

ACK 7c57965

@afilini afilini merged commit cf13c80 into bitcoindevkit:master Aug 31, 2022
@afilini afilini mentioned this pull request Sep 16, 2022
2 tasks
notmandatory added a commit that referenced this pull request Sep 26, 2022
e1fa0b6 Fix the new release process (Alekos Filini)

Pull request description:

  ### Description

  A few things I noticed while doing the 0.22 release.

  Follow-up of #544

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)

ACKs for top commit:
  notmandatory:
    ACK e1fa0b6

Tree-SHA512: 5e60e73ba1f820fc39f62b75583e1125f911e578ab7263a20636e2a995d0efa10ba1b3b66a1e03d8a2ed61e32c00b53de9d6bbdb31de94db37c79fa5acdbf483
@notmandatory notmandatory deleted the doc/fix_patch branch October 24, 2022 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
5 participants