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

avoid messing up changelog history on each subsequent release #14025

Closed

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Apr 19, 2020

  • I've updated and simplified the changelog instructions
    instead of destroying changelog history (eg git blame etc) on each release by moving changelog.md to changelog_x_y_z.md and replacing changelog.md upon each release, we are now directly writing changelog entries to the expected final destination. changelog.md is now just a pointer to the latest changelog + showing where to find instructions.
  • so we're now writing current changelog entries to changelogs/changelog_1_4_0.md instead of changelog.md. Even if 1.4.0 ends up being something else (eg 2.0.0), the git
    history won't be destroyed because git will recognize this as a move, unlike what's been happening to changelogs before this PR because changelog.md keeps getting overwritten
  • renamed changelogs/changelog_0_18_1.md => changelogs/changelog_0_18_0.md since that's what's on the website and 0.18.1 was not a release
  • updated changelogs/readme.md with latest entries

note to reviewer

when merging, please "rebase and merge" instead of "squash and merge" so that, at least in git history, git will recognize changelogs/changelog_1_4_0.md as a move (as you can see with https://patch-diff.githubusercontent.com/raw/nim-lang/Nim/pull/14025.patch); git is not that smart about renames followed by re-introduction (with new content) of orginial file that was moved. It won't show as a move in github diff because of isaacs/github#900 but at least it will in git history)

@timotheecour timotheecour changed the title avoid messing up changelog history on each release avoid messing up changelog history on each subsequent release Apr 19, 2020
@timotheecour timotheecour force-pushed the pr_fix_changelog_files branch from 7a992a8 to 51075ce Compare April 19, 2020 22:35
@Clyybber
Copy link
Contributor

This is IMO unneccessary complexity and makes backporting more cumbersome.

@timotheecour
Copy link
Member Author

timotheecour commented Apr 20, 2020

This is IMO unneccessary complexity and makes backporting more cumbersome.

not that's the exact opposite: less complexity and easier backport.
no files being moved around and renamed on each release, git history isn't destroyed, and if you backport a commit that changes a changelog entry, you'll get an easier to resolve merge conflict:

  • deleted by us: changelogs/changelog_1_4_0.md
  • instead of both modified: changelog.md

So with current approach which is to generate a final changelog for backported releases by aggregating commit messages, all you need is to git rm changelogs/changelog_1_4_0.md whenever such conflict occurs

Copy link
Member

@narimiran narimiran left a comment

Choose a reason for hiding this comment

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

I've already begged you to slow down with your pull requests, so we don't have to play this ping-pong and burn both CI cycles and our time on reviewing the silly mistakes you make in a hurry.

Please, your time is not more valuable than others'.


Now, as for the content of this PR: I can see the benefits, but I can see some downsides too. I'm not convinced this is the way to go.

changelog.md Outdated Show resolved Hide resolved
changelogs/changelog_X_XX_X.md Show resolved Hide resolved
changelogs/readme.md Outdated Show resolved Hide resolved
changelogs/readme.md Outdated Show resolved Hide resolved
@timotheecour timotheecour force-pushed the pr_fix_changelog_files branch 2 times, most recently from eeb2953 to cf71301 Compare April 21, 2020 01:01
@timotheecour
Copy link
Member Author

@Araq
Copy link
Member

Araq commented Apr 21, 2020

@narimiran I'm in favour of the idea behind this change fwiw.

@timotheecour timotheecour force-pushed the pr_fix_changelog_files branch from cf71301 to d1c3c55 Compare April 21, 2020 19:58
@timotheecour timotheecour force-pushed the pr_fix_changelog_files branch from d1c3c55 to cfe6643 Compare April 21, 2020 22:27
@krux02
Copy link
Contributor

krux02 commented Apr 23, 2020

I do see the problem. Keeping old pull requests active causes really annoying problems each release when there are changelog entries. But I don't agree on the solution.

Why do you disrupt the workflow?
Why is there such a big diff?
Why don't you make changelog.md just a symlink?

Copy link
Contributor

@krux02 krux02 left a comment

Choose a reason for hiding this comment

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

Why did you rename changelogs/changelog_0_18_1.md → changelogs/changelog_0_18_0.md?

* https://nim-lang.org/blog/2017/05/17/version-0170-released.html
* https://nim-lang.org/blog/2017/09/07/version-0172-released.html
* https://nim-lang.org/blog/2018/03/01/version-0180-released.html
* https://nim-lang.org/blog/2018/09/26/version-0190-released.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

- The `define` and `undef` pragmas have been de-deprecated.

## Tool changes

Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't I see File renamed without changes. here?

Copy link
Member Author

Choose a reason for hiding this comment

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

this was explained in PR msg.
see https://patch-diff.githubusercontent.com/raw/nim-lang/Nim/pull/14025.patch

---
 changelog.md => changelogs/changelog_1_4_0.md |  0
 changelogs/changelog_X_XX_X.md                |  3 +--
 changelogs/readme.md                          | 27 +++++++++++++++----
 3 files changed, 23 insertions(+), 7 deletions(-)
 rename changelog.md => changelogs/changelog_1_4_0.md (100%)

@juancarlospaco
Copy link
Collaborator

juancarlospaco commented Apr 23, 2020

I dont understand the diff, it should not be that big, old changelogs should be left untouched.
Symlinks wont work on all Windows AFAIK.

Alternatively you should steal the idea from https://github.com/juancarlospaco/changelogen#changelogen
Try it on your own fork, no merge conflicts on changelog.md all the time,
automatic links to diff per commit, arbitrary categories, JSON representation if needed, and other benefits.

@Araq
Copy link
Member

Araq commented Apr 23, 2020

Way too complex. Instead we could have changelog_1_3.md instead of changelog.md. A final rename step remains when we do the release.

@Araq
Copy link
Member

Araq commented Apr 24, 2020

Rejected for now, we should find a better way if still a problem after your .git config change.

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.

6 participants