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

Fix changelog item generation #8945

Merged
merged 2 commits into from Oct 26, 2020
Merged

Conversation

dvandersluis
Copy link
Member

@dvandersluis dvandersluis commented Oct 26, 2020

Re #8930. I found a couple issues when using rake changelog:fix to write a changelog item for another PR. Foremost, [Fix #xxxx] as requested in the contributing guidelines was not being parsed properly (it was only looking for Fixes, now it accepts both). tasks/changelog.rb did not have a spec, so I added a basic one that just covers this issue but is not exhaustive.

I also updated the tests in project_spec.rb:

The last item is potentially contentious because the rake task prefaces items with * [#x](https://github.com/rubocop-hq/rubocop/pull/x): if a ref ID cannot be found, which will then cause bundle exec rake default to fail. This is kind of a paradox if there isn't already an issue to refer to (ie. sometimes in practice the link is to the PR that merges the change, which doesn't exist before rake default is run), so I'm not 100% sure about this, but I think the test is valuable in general to ensure we don't end up with [#x] in the changelog.

/cc @marcandre


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@marcandre marcandre self-assigned this Oct 26, 2020
@marcandre
Copy link
Contributor

tasks/changelog.rb did not have a spec, so I added a basic one that just covers this issue but is not exhaustive.

Oh, right, I forgot to copy it over from rubocop-ast. Done in #8946.

You changes are great. How about moving your spec to spec/tasks/changelog_spec.rb?

spec/spec_helper.rb Outdated Show resolved Hide resolved
@@ -51,7 +51,7 @@ def last_commit_title
end

def extract_id(body)
/^\[Fixes #(\d+)\] (.*)/.match(body)&.captures || [nil, body]
/^\[Fix(?:es)? #(\d+)\] (.*)/.match(body)&.captures || [nil, body]
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, didn't know we could write either 😅

- Properly handle issues referring to other rubocop-hq repos
- Only evaluate links at the beginning of a changelog item
- Ensure that the link reference is valid (eg. not [#x] or [])
- Ensure that a changelog item does not contain [Fix(es) #...]
- Fixed improper CHANGELOG.md item
@dvandersluis
Copy link
Member Author

@marcandre updated, merged my spec into yours, thanks!

Copy link
Contributor

@marcandre marcandre left a comment

Choose a reason for hiding this comment

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

Good job 💪 ! Thanks 😄

@mergify mergify bot merged commit fb377fa into rubocop:master Oct 26, 2020
@dvandersluis dvandersluis deleted the fix/changelog branch January 18, 2021 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants