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 1.4 release section on CHANGELOG.md #3400

Closed
wants to merge 2 commits into from
Closed

Add 1.4 release section on CHANGELOG.md #3400

wants to merge 2 commits into from

Conversation

thehowl
Copy link
Contributor

@thehowl thehowl commented Jan 19, 2018

Pretty much copied over from go-gitea/blog#54, although reduced and with jokes removed. There is a comment for the blogpost, which will be changed once it's out and Gitea is released.

Here's how I thought of handling patch updates: at the end of the 1.4 section, add a second sub-heading ## [1.4.1](https://...), which will list all the merged PRs (since if they were so important to deserve a patch update and a backport, they should probably be mentioned.)

@strk
Copy link
Member

strk commented Jan 19, 2018

Great work ! LGTM.
Please remove the blog post link though, doesn't really make sense

I think changelog should be readable with a text pager, so 80 cols wrap would also be cool

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 19, 2018
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@05ab747). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3400   +/-   ##
=========================================
  Coverage          ?   35.65%           
=========================================
  Files             ?      281           
  Lines             ?    40555           
  Branches          ?        0           
=========================================
  Hits              ?    14458           
  Misses            ?    23964           
  Partials          ?     2133

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05ab747...d16b1c9. Read the comment docs.

@lafriks
Copy link
Member

lafriks commented Jan 19, 2018

Also links to PR are not needed #PULLNR is enough

Copy link
Member

@techknowlogick techknowlogick 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 inconsistent with past changelog entries.

Previous changelog sections are broken up into subsections like BREAKING, FEATURE, BUGFIX, etc..

I know we don't autogenerate the changelog yet, but there is the tool: https://github.com/go-gitea/changelog that could eventually do it.

I think the additional commentary for each item can be kept in the blog post. I don't mean to sound "but we've always done it this way" as I'm sure I probably do, I really appreciate the work you've done (as I'm sure everyone else does). I just think consistency is key in this case.

EDIT: If an owner or other maintainers disagree with me then please ignore this review and merge away, as perhaps I am the only dissenting voice in this situation and I don't feel too strongly about these opinions.

@lafriks
Copy link
Member

lafriks commented Jan 20, 2018

I also think that changelog should be kept more technical

@thehowl
Copy link
Contributor Author

thehowl commented Jan 20, 2018

I can see the point in that and I agree with @tecknowlogick, however one big issue I see with the current changelog system is there is a lot of noise. Do users care about refactoring PRs? Do they care about bugfixes for features introduced in the same release? Do they care about fixed typos?

If we're gonna keep the current style of changelog for CHANGELOG.md, at least the noise should be cut down manually. Reading long lists is a daunting task, and especially boring if there are a lot of things you don't care about.

@thehowl
Copy link
Contributor Author

thehowl commented Jan 20, 2018

Please remove the blog post link though, doesn't really make sense

The point of that was to provide nevertheless a way to see a changelog with a more human touch, whilst keeping the Changelog file technical, as also said by lafriks and techknowlogick.

@lafriks
Copy link
Member

lafriks commented Jan 20, 2018

I also agree that changelog noise must be kept down. Might be good to introduce new label and include only these PR in changelog that have this label

@thehowl
Copy link
Contributor Author

thehowl commented Jan 20, 2018

Or a blacklist: PRs with label changelog/minor not included in the changelog for the release. Either way, looks like this PR won't be going anywhere, so closing.

@thehowl thehowl closed this Jan 20, 2018
@strk
Copy link
Member

strk commented Jan 20, 2018 via email

@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants