Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Changelog for 0.29 missed breaking change #1368

Closed
victorb opened this issue May 29, 2018 · 7 comments
Closed

Changelog for 0.29 missed breaking change #1368

victorb opened this issue May 29, 2018 · 7 comments

Comments

@victorb
Copy link
Member

victorb commented May 29, 2018

In the latest release (0.29 - https://github.com/ipfs/js-ipfs/releases/tag/v0.29.0) there is only one breaking change while in reality there is two, "changes peer prop in return value from swarm.peers to be a PeerId (#1252) (e174866)" also changes the API and should therefore be considered a breaking change.

@daviddias
Copy link
Member

@victorbjelkholm mind updating it? Also, better point it to the release highlights #1320

@victorb
Copy link
Member Author

victorb commented May 29, 2018

I now realize there is more changes, listed under the "release highlights" as "API Changes" but not existing in the actual changelog. Should just copy-paste that into the changelog.

Both the tag + the commit has been signed by you and has a signoff from @alanshaw, so not sure if editing is the right way to go here... Should we just change the GitHub release and make a new commit instead? Won't be included in the npm package in that case though, unless we do another release...

@daviddias
Copy link
Member

I've added the release highlights to the top so that users that are coming and checking the changelog can find all the highlights.

Can you do a post-mortem on why the changelog failed and what needs to happen to fix it?

@victorb
Copy link
Member Author

victorb commented May 29, 2018

post-mortem

When reviewing PRs, we failed to identify breaking changes and accurately review the commit messages so breaking changes are highlighted as such. This made our automatic release process miss listing those changes as "Breaking Changes" in our changelog.

The changes were accurately described as breaking changes in our release issue, but we failed to move them over to the changelog before actually releasing, leading to us missing them again.

In the future, to prevent this issue, we will:

  • Make sure to review changes and have breaking changes in mind, so we can leave feedback when commit messages have to be changed, so our automatic tools can pick up the changes correctly.
  • If "API Changes" exists in the release issue, make sure those commits are accurately described as breaking changes, so the automatic tools can list them appropriately.

On that note, I'm wondering if there is a way we can capture this automatically. Brainstorming an idea, I can imagine that we can use changes in interface-ipfs-core as a indicator that the API has changed, but only if there is modifications, not additions, and then flagging that somehow to make manual reviews and making sure the commits are correctly tagged?

@daviddias
Copy link
Member

@victorbjelkholm with ipfs/aegir#225 in mind, it would actually be good to have a PR for the release with a special commit message that would give us access to the changelog for review before merging the release commit.

@alanshaw
Copy link
Member

That would be great, I was going to suggest a dry run where you can see what's going to be done before it all happens, but a PR would give us that preview step and also allow us to tweak things before actual release.

@daviddias daviddias added the status/ready Ready to be worked label May 30, 2018
@alanshaw
Copy link
Member

alanshaw commented May 30, 2018

I've sent a PR to amend the changelog here #1370

I'll be more vigilant when reviewing PRs in the future to ensure commits feature "BREAKING CHANGE" message when breaking changes are made so that it can be picked up automatically by aegir.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants