-
Notifications
You must be signed in to change notification settings - Fork 51
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
rules: skip line-length rule for URLs and quoted lines #30
Changes from 2 commits
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 |
---|---|---|
|
@@ -31,6 +31,12 @@ module.exports = { | |
var failed = false | ||
for (let i = 0; i < parsed.body.length; i++) { | ||
const line = parsed.body[i] | ||
// Skip quoted lines, e.g. for original commit messages of V8 backports. | ||
if (line.startsWith(' ')) | ||
continue | ||
// Skip lines with URLs. | ||
if (line.match(/https?:\/\//)) | ||
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 think RegExp#test would be better here. 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. @thefourtheye There’s 0 difference here, right? Anyway, done! 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 are correct, there will be no difference. I was going for idiomatic code 🙂 |
||
continue | ||
if (line.length > len) { | ||
failed = true | ||
context.report({ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,5 +67,69 @@ ${'aaa'.repeat(30)}` | |
tt.end() | ||
}) | ||
|
||
t.test('quoted lines', (tt) => { | ||
const v = new Validator() | ||
const context = new Commit({ | ||
sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea' | ||
, author: { | ||
name: 'Evan Lucas' | ||
, email: '[email protected]' | ||
, date: '2016-04-12T19:42:23Z' | ||
} | ||
, message: `src: make foo mor foo-ey | ||
|
||
Here’s the original code: | ||
|
||
${'aaa'.repeat(30)} | ||
|
||
That was the original code. | ||
` | ||
}, v) | ||
|
||
context.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(context, { | ||
options: { | ||
length: 72 | ||
} | ||
}) | ||
tt.end() | ||
}) | ||
|
||
t.test('URLs', (tt) => { | ||
const v = new Validator() | ||
const context = new Commit({ | ||
sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea' | ||
, author: { | ||
name: 'Evan Lucas' | ||
, email: '[email protected]' | ||
, date: '2016-04-12T19:42:23Z' | ||
} | ||
, message: `src: make foo mor foo-ey | ||
|
||
https://${'very-'.repeat(80)}-long-url.org/ | ||
` | ||
}, v) | ||
|
||
context.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(context, { | ||
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.
Shouldn't this have four space characters? The test actually tests with four space characters.
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’m not sure … switched to 4 spaces for now, yes