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 linter for CHANGELOG formatting [ci skip] #45674

Merged
merged 1 commit into from Aug 3, 2022

Conversation

skipkayhil
Copy link
Member

Summary

There have been a number of recent commits introducing incorrectly
formatted CHANGELOG entries:

  • 9f0b8eb was missing an author
  • 936a862 had trailing whitespace
  • 238432d had wrong number of leading whitespace
  • 51852d2 had wrong number of leading whitespace in the header

To prevent these inconsistencies from happening in the future, I wrote a
small linter for CHANGELOG files that catches all of the above errors.

Other Information

  • is this something rails/rails wants running on Pull Requests? Currently I have it running as a daily job in my repo but it would definitely be more effective on PRs
  • if yes, where should it go? My repo/rails/rails/rails/?/a gem?

Very open to feedback on the approach and would appreciate any suggestions!

cc @yahonda ref #45559 (comment)

There have been a number of recent commits introducing incorrectly
formatted CHANGELOG entries:
- 9f0b8eb was missing an author
- 936a862 had trailing whitespace
- 238432d had wrong number of leading whitespace
- 51852d2 had wrong number of leading whitespace in the header

To prevent these inconsistencies from happening in the future, I wrote a
small linter for CHANGELOG files that catches all of the above errors.
@yahonda
Copy link
Member

yahonda commented Jul 28, 2022

I would like to see how this job finds these issues, such as missing authors and extra whitespaces. Would you push some "bad" changelog entries to see how it works? Of course, we will remove/revert the "bad" entries before merging.

@skipkayhil skipkayhil changed the title Add linter for CHANGELOG formatting Add linter for CHANGELOG formatting [ci skip] Jul 28, 2022
@yahonda
Copy link
Member

yahonda commented Jul 28, 2022

Confirmed https://github.com/rails/rails/runs/7551769464?check_suite_focus=true and it looks good to me.

@yahonda
Copy link
Member

yahonda commented Jul 28, 2022

Let me answer these questions.

is this something rails/rails wants running on Pull Requests? Currently I have it running as a daily job in my repo but it would definitely be more effective on PRs

Yes. I think running this job on pull requests is fine so that we can ask contributors to address offenses in the CHANGELOG.

if yes, where should it go? My repo/rails/rails/rails/?/a gem?

I think under rails/rails https://github.com/rails/rails is fine.
If you mean where "https://github.com/skipkayhil/rails-bin" goes, I think you can leave https://github.com/skipkayhil/rails-bin as it is.

@yahonda yahonda self-requested a review July 29, 2022 05:26
@yahonda
Copy link
Member

yahonda commented Jul 29, 2022

I think we can revert 3fcf282 and drop these two commits from this pull request. We can see how this job detects offenses below.

https://github.com/rails/rails/runs/7551769464?check_suite_focus=true

@yahonda
Copy link
Member

yahonda commented Aug 1, 2022

Let's open this pull request for a couple of more days to see if there are any other feedback.

@yahonda yahonda merged commit c558d98 into rails:main Aug 3, 2022
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.

None yet

2 participants