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 breaking-changes section to changelog #530

Merged
merged 6 commits into from Oct 10, 2017

Conversation

Projects
None yet
7 participants
@bastelfreak
Contributor

bastelfreak commented Jun 24, 2017

Hi,
this PR allows us to create another section in the changelog, for breaking changes. This is not a 100% implementation of #316, but may help people. Tested this successfully on https://github.com/voxpupuli/puppet-dhcp/blob/master/CHANGELOG.md

@olleolleolle

I like this feature.

Q: If a PR is marked as both Breaking and an Enhancement, it would be in both lists, right?

@olleolleolle

This comment has been minimized.

Show comment
Hide comment
@olleolleolle

olleolleolle Jun 24, 2017

Collaborator

Looking at the generated CHANGELOG.md, I feel that it'd be cool to place any breaking changes section FIRST.

Collaborator

olleolleolle commented Jun 24, 2017

Looking at the generated CHANGELOG.md, I feel that it'd be cool to place any breaking changes section FIRST.

@olleolleolle

This comment has been minimized.

Show comment
Hide comment
@olleolleolle

olleolleolle Jun 24, 2017

Collaborator

So, there's room for careful refactoring - or disabling a RuboCop check to pass this:

lib/github_changelog_generator/generator/generator.rb:105:5: C: Metrics/CyclomaticComplexity: Cyclomatic complexity for parse_by_sections is too high. [8/7]
    def parse_by_sections(issues, pull_requests)
    ^^^

Collaborator

olleolleolle commented Jun 24, 2017

So, there's room for careful refactoring - or disabling a RuboCop check to pass this:

lib/github_changelog_generator/generator/generator.rb:105:5: C: Metrics/CyclomaticComplexity: Cyclomatic complexity for parse_by_sections is too high. [8/7]
    def parse_by_sections(issues, pull_requests)
    ^^^

@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Jun 24, 2017

Contributor

I tried to refactor it, but didn't find a clean solution. If anybody has ideas please let me know. I will try to place the section at the beginning.

Contributor

bastelfreak commented Jun 24, 2017

I tried to refactor it, but didn't find a clean solution. If anybody has ideas please let me know. I will try to place the section at the beginning.

@olleolleolle

This comment has been minimized.

Show comment
Hide comment
@olleolleolle

olleolleolle Jun 24, 2017

Collaborator

@bastelfreak Failing forward is also a good option!

Add a # rubocop:disable Metrics/CyclomaticComplexity as a comment on the line of the def of the offending method; and the crypt keepers will get to this gnarly bit at some point in the future.

Don't worry about it.

Collaborator

olleolleolle commented Jun 24, 2017

@bastelfreak Failing forward is also a good option!

Add a # rubocop:disable Metrics/CyclomaticComplexity as a comment on the line of the def of the offending method; and the crypt keepers will get to this gnarly bit at some point in the future.

Don't worry about it.

@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Jun 24, 2017

Contributor

@olleolleolle I reordered the output, the breaking stuff should now be the first.

Contributor

bastelfreak commented Jun 24, 2017

@olleolleolle I reordered the output, the breaking stuff should now be the first.

@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Jul 19, 2017

Contributor

Hi @olleolleolle, what needs to be done to get this merged?

Contributor

bastelfreak commented Jul 19, 2017

Hi @olleolleolle, what needs to be done to get this merged?

@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Jul 19, 2017

Contributor

I made some tests again and I'm not 100% if it still works after I changed the ordering. I will have a look tomorrow.

Contributor

bastelfreak commented Jul 19, 2017

I made some tests again and I'm not 100% if it still works after I changed the ordering. I will have a look tomorrow.

@hunner

This comment has been minimized.

Show comment
Hide comment
@hunner

hunner Aug 10, 2017

Contributor

Q: If a PR is marked as both Breaking and an Enhancement, it would be in both lists, right?

No, backwards-incompatible (or "breaking") changes are always features because you are enhancing the project with behavior that is desirable, so it would not make sense to put them in both sections.

For a standardized workflow around this, see http://keepachangelog.com/ . It's great, because it tries to make a portable standardized changelog format that works anywhere, and has a formal definition for what goes in each section and how to adhere to http://semver.org/ .

Contributor

hunner commented Aug 10, 2017

Q: If a PR is marked as both Breaking and an Enhancement, it would be in both lists, right?

No, backwards-incompatible (or "breaking") changes are always features because you are enhancing the project with behavior that is desirable, so it would not make sense to put them in both sections.

For a standardized workflow around this, see http://keepachangelog.com/ . It's great, because it tries to make a portable standardized changelog format that works anywhere, and has a formal definition for what goes in each section and how to adhere to http://semver.org/ .

@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Sep 25, 2017

Contributor

Hi @olleolleolle @skywinder could you recheck this PR?
I fixed one issue and rebased against latest master.

Contributor

bastelfreak commented Sep 25, 2017

Hi @olleolleolle @skywinder could you recheck this PR?
I fixed one issue and rebased against latest master.

@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Sep 30, 2017

Contributor

Hi @olleolleolle, can you check this PR as well? Would be awesome!

Contributor

bastelfreak commented Sep 30, 2017

Hi @olleolleolle, can you check this PR as well? Would be awesome!

@olleolleolle

This comment has been minimized.

Show comment
Hide comment
@olleolleolle

olleolleolle Sep 30, 2017

Collaborator
  • Style Feedback: Avoid using push, use "the shovel operator" (<<) to append to lists.
  • Design note: Avoid adding more repetition: Find a way to not add a fourth repeat of things which are already too long
    • example: bugs_a, enhancement_a, breaking_a, issues_a = - this calls out for some sort of structure
    • example continued: generate_sub_section repeated once per those things, each with a named option
    • example continued once more: parse_by_sections also has 4 of this thing
Collaborator

olleolleolle commented Sep 30, 2017

  • Style Feedback: Avoid using push, use "the shovel operator" (<<) to append to lists.
  • Design note: Avoid adding more repetition: Find a way to not add a fourth repeat of things which are already too long
    • example: bugs_a, enhancement_a, breaking_a, issues_a = - this calls out for some sort of structure
    • example continued: generate_sub_section repeated once per those things, each with a named option
    • example continued once more: parse_by_sections also has 4 of this thing
@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Sep 30, 2017

Contributor
  • replaced push with shovel operator
  • rebased
Contributor

bastelfreak commented Sep 30, 2017

  • replaced push with shovel operator
  • rebased

bastelfreak added some commits Jun 24, 2017

@bastelfreak

This comment has been minimized.

Show comment
Hide comment
@bastelfreak

bastelfreak Oct 10, 2017

Contributor

@olleolleolle I added the commit from @ekohl, could you take a look again?

Contributor

bastelfreak commented Oct 10, 2017

@olleolleolle I added the commit from @ekohl, could you take a look again?

@olleolleolle

This comment has been minimized.

Show comment
Hide comment
@olleolleolle

olleolleolle Oct 10, 2017

Collaborator

@bastelfreak Thanks for that.

Could you fix the lint complaints?

Collaborator

olleolleolle commented Oct 10, 2017

@bastelfreak Thanks for that.

Could you fix the lint complaints?

bastelfreak added some commits Oct 10, 2017

@olleolleolle

This comment has been minimized.

Show comment
Hide comment
@olleolleolle

olleolleolle Oct 10, 2017

Collaborator

@bastelfreak Thanks for passing lint checks, I can merge this from here.

Collaborator

olleolleolle commented Oct 10, 2017

@bastelfreak Thanks for passing lint checks, I can merge this from here.

@olleolleolle olleolleolle merged commit 21ec2db into github-changelog-generator:master Oct 10, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+1.9%) to 73.409%
Details

@bastelfreak bastelfreak deleted the bastelfreak:breaking branch Oct 10, 2017

@nfischer

This comment has been minimized.

Show comment
Hide comment
@nfischer

nfischer Jan 18, 2018

Should this be documented on the README?

nfischer commented Jan 18, 2018

Should this be documented on the README?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment