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

Add a Changelog #889

Merged
merged 32 commits into from
Apr 30, 2020
Merged

Add a Changelog #889

merged 32 commits into from
Apr 30, 2020

Conversation

luleyleo
Copy link
Collaborator

@luleyleo luleyleo commented Apr 29, 2020

This introduces an in-tree changelog base on Keep a Changelog.

Whenever a new PR ist created it should contain changelog entries for everything it changes, thus keeping the changelog always in sync with master.

Every changelog entry is annotated with the PR that introduced it and the author for attribution.

The attribution is also the reason why the Maintenance section exists, even though entries in it have no direct impact on users.

Annotating entries with a link to the introducing PR causes a chicken-and-egg problem tho.
When creating the PR you wont know the number yet, which you need to refer to it.
Having a quick link to the related PR seems very useful however as it provides much more information about the change than the changelog ever could.

See Zulip for previous discussion.

@luleyleo luleyleo added the S-needs-review waits for review label Apr 29, 2020
@xStrom
Copy link
Member

xStrom commented Apr 29, 2020

The chicken and egg problem isn't that bad because github issue numbers are sequential and you can guess what number you will get. Sometimes you lose the race, but that should be rare.

@luleyleo
Copy link
Collaborator Author

Right, they are sequential that makes it a bit better but then you would have to figure out what the current count is at, I wonder if there is like a 'next PR number' badge that we could add to the readme for example.

Copy link
Member

@xStrom xStrom left a comment

Choose a reason for hiding this comment

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

Generally looks good, thanks for taking this on! I made some inline comments about improvements.

Also do a search & replace for my name: xStorm -> xStrom.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@luleyleo
Copy link
Collaborator Author

@xStrom I've fixed your name (sorry for that), added some other things I've missed and hopefully corrected the tenses.

I'm not sure if we should include changes to internal-but-public API such as InternalEvent. For now I've just added them.

@xStrom
Copy link
Member

xStrom commented Apr 30, 2020

We should also add a section/note to CONTRIBUTING.md about needing to change the changelog in PRs.

I think documenting the internal stuff is fine while we're in the 0.x versions. We can reconsider this later, e.g. once we hit 1.0.

Also #890 got merged.

@luleyleo
Copy link
Collaborator Author

I've added it to CONTRIBUTING.md.

I've not added anything regarding #890 as it is a follow-up for a feature that is not released yet.
I think keeping the changelog compact and only freezing entries once the version is released makes it easier to read. For example I've merged the Event::MouseLeave addition with your effort to collect InternalEvents. Thus there is only one entry mentioning the addition of InternalEvent::MouseLeave.

@xStrom
Copy link
Member

xStrom commented Apr 30, 2020

I think that makes sense. We don't want the changelog to be a mirror copy of the merge commit log. Multiple PRs working on the same conceptual feature can be streamlined into a single entry in the changelog.

CHANGELOG.md Outdated Show resolved Hide resolved
@xStrom
Copy link
Member

xStrom commented Apr 30, 2020

For completeness sake, let's add all the previous versions to the changelog as well. I know we don't have the the actual changes for them right now, but we could at least have the version numbers and their dates.

@luleyleo
Copy link
Collaborator Author

I've added them, left out 3.3.3 however as it was yanked anyways.

CHANGELOG.md Show resolved Hide resolved
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

This is great, thanks for taking it on!

Copy link
Member

@xStrom xStrom left a comment

Choose a reason for hiding this comment

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

Good to go! 🥳

@luleyleo luleyleo merged commit 9768b92 into linebender:master Apr 30, 2020
@xStrom xStrom removed the S-needs-review waits for review label May 15, 2020
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.

3 participants