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

gitlint: Add regex to ignore some long lines. #1509

Merged

Conversation

neiljp
Copy link
Collaborator

@neiljp neiljp commented Jun 1, 2024

What does this PR do, and why?

This was motivated by the failure of a CI run on #1494, where I added a URL to the commit body.

This does not resolve issues for all previous commits where the B1 rule of long lines in the body would fail, but will allow some obvious common cases to pass.

Inspired by jorisroovers/gitlint#255, this should cover at least lines starting with:

  • http or https [where URLs can be long]
  • Co-authored-by: [where emails/names can be long]

The downside to this rule is that the line numbering is apparently offset, based on the gitlint documentation.
(https://jorisroovers.com/gitlint/latest/ignoring_commits/)

This was tested manually on the last 1500 commits on main, and limits the number of gitlint errors as expected for URLs. No changes are noted for Co-authored-by: at this time.

External discussion & connections

  • Discussed in #zulip-terminal in topic
  • Fully fixes #
  • Partially fixes issue #
  • Builds upon previous unmerged work in PR #
  • Is a follow-up to work in PR #
  • Requires merge of PR #
  • Merge will enable work on #

How did you test this?

  • Manually - Behavioral changes
  • Manually - Visual changes
  • Adapting existing automated tests
  • Adding automated tests for new behavior (or missing tests)
  • Existing automated tests should already cover this (only a refactor of tested code)

Self-review checklist for each commit

  • It is a minimal coherent idea
  • It has a commit summary following the documented style (title & body)
  • It has a commit summary describing the motivation and reasoning for the change
  • It individually passes linting and tests
  • It contains test additions for any new behavior
  • It flows clearly from a previous branch commit, and/or prepares for the next commit

This does not resolve issues for all previous commits where the B1 rule
of long lines in the body would fail, but will allow some obvious common
cases to pass.

Inspired by jorisroovers/gitlint#255, this should cover at least lines
starting with:
- http or https [where URLs can be long]
- Co-authored-by: [where emails/names can be long]

The downside to this rule is that the line numbering is apparently
offset, based on the gitlint documentation.
(https://jorisroovers.com/gitlint/latest/ignoring_commits/)
@neiljp neiljp added the area: infrastructure Project infrastructure label Jun 1, 2024
@zulipbot zulipbot added the size: XS [Automatic label added by zulipbot] label Jun 1, 2024
@neiljp neiljp added this to the Next Release milestone Jun 1, 2024
@neiljp neiljp merged commit 176067f into zulip:main Jun 1, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: infrastructure Project infrastructure size: XS [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants