-
-
Notifications
You must be signed in to change notification settings - Fork 53
feat: support co-authorship lines in body #130
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
base: main
Are you sure you want to change the base?
Changes from all commits
37e4e88
f1496de
f778e67
5004df9
d7c76aa
dc6b810
f05715d
69779f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -129,5 +129,60 @@ https://${'very-'.repeat(80)}-long-url.org/ | |||||||||||||
| tt.end() | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| t.test('Co-author lines', (tt) => { | ||||||||||||||
| const sha = 'f1496de5a7d5474e39eafaafe6f79befe5883a5b' | ||||||||||||||
| const author = { | ||||||||||||||
| name: 'Jacob Smith', | ||||||||||||||
| email: '3012099+JakobJingleheimer@users.noreply.github.com', | ||||||||||||||
| date: '2025-12-22T09:40:42Z' | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const v = new Validator() | ||||||||||||||
| const overlongMessage = `fixup!: apply case-insensitive suggestion | ||||||||||||||
| Co-authored-by: Michaël Zasso <37011812+targos@users.noreply.github.com>` | ||||||||||||||
|
Comment on lines
+141
to
+142
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test-case is asserting it correctly errors. Your proposed change would cause it to not error (and then it's the same case as the other one added in this PR).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would it not error? Mind the lack of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The (lack of) leading spaces
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand how relevant are the leading spaces for the code you're adding, it feels much more likely to me that someone would forgot to add / remove by mistake the colon rather than add leading spaces. Anyway, I'm just trying to suggest something to help you get a green CI, feel free to fix the test another way
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You requested this test-case though. If you don't want it, I'll just remove it and CI will be green.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I did, removing it sounds good to me |
||||||||||||||
| const bad = new Commit({ | ||||||||||||||
| sha, | ||||||||||||||
| author, | ||||||||||||||
| message: overlongMessage | ||||||||||||||
| }, v) | ||||||||||||||
|
|
||||||||||||||
| bad.report = (opts) => { | ||||||||||||||
| tt.pass('called report') | ||||||||||||||
| tt.equal(opts.id, 'line-length', 'id') | ||||||||||||||
| tt.equal(opts.string, overlongMessage.split('\n')[1], 'string') | ||||||||||||||
| tt.equal(opts.level, 'fail', 'level') | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| Rule.validate(bad, { | ||||||||||||||
| options: { | ||||||||||||||
| length: 72 | ||||||||||||||
| } | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| const good = new Commit({ | ||||||||||||||
| sha, | ||||||||||||||
| author, | ||||||||||||||
| message: [ | ||||||||||||||
| 'fixup!: apply case-insensitive suggestion', | ||||||||||||||
| 'Co-authored-by: Michaël Zasso <37011812+targos@users.noreply.github.com>' | ||||||||||||||
| ].join('\n') | ||||||||||||||
| }, v) | ||||||||||||||
|
|
||||||||||||||
| good.report = (opts) => { | ||||||||||||||
| tt.pass('called report') | ||||||||||||||
| tt.equal(opts.id, 'line-length', 'id') | ||||||||||||||
| tt.equal(opts.string, '', 'string') | ||||||||||||||
| tt.equal(opts.level, 'pass', 'level') | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| Rule.validate(good, { | ||||||||||||||
| options: { | ||||||||||||||
| length: 72 | ||||||||||||||
| } | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| tt.end() | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| t.end() | ||||||||||||||
| }) | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'm not sure it's worth doing, but we could have a shared util to avoid duplicating the regex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 I think if we need it in a third place, then it's worth having a shared util.