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

feat: add --validate-no-metadata flag #125

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/cmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const knownOpts = {
help: Boolean,
version: Boolean,
'validate-metadata': Boolean,
'validate-no-metadata': Boolean,
tap: Boolean,
out: path,
list: Boolean,
Expand Down
11 changes: 11 additions & 0 deletions lib/rules/pr-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ export default {
defaults: {},
options: {},
validate: (context, rule) => {
if (rule.options.expectsMissing) {
context.report({
id,
message: 'Commit must not have a PR-URL.',
string: context.prUrl,
line: 0,
column: 0,
level: context.prUrl ? 'fail' : 'pass'
})
return
}
if (!context.prUrl) {
context.report({
id,
Expand Down
10 changes: 10 additions & 0 deletions lib/rules/reviewers.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ export default {
return
}

if (rule.options.expectsMissing) {
return context.report({
id,
message: 'Commit must not have any reviewer.',
string: null,
line: 0,
column: 0,
level: parsed.reviewers.length ? 'fail' : 'pass'
})
}
if (!Array.isArray(parsed.reviewers) || !parsed.reviewers.length) {
// See nodejs/node#5aac4c42da104c30d8f701f1042d61c2f06b7e6c
// for an example
Expand Down
5 changes: 4 additions & 1 deletion lib/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ export default class ValidateCommit extends EE {
for (const key of keys) {
this.rules.set(key, new BaseRule(RULES[key]))
}
if (!this.opts['validate-metadata']) {
if (this.opts['validate-no-metadata']) {
this.rules.get('pr-url').options.expectsMissing = true
this.rules.get('reviewers').options.expectsMissing = true
} else if (!this.opts['validate-metadata']) {
this.disableRule('pr-url')
this.disableRule('reviewers')
this.disableRule('metadata-end')
Expand Down
12 changes: 6 additions & 6 deletions test/rules/pr-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test('rule: pr-url', (t) => {
}
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.test('invalid numeric', (tt) => {
Expand All @@ -43,7 +43,7 @@ test('rule: pr-url', (t) => {
}
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.test('invalid', (tt) => {
Expand All @@ -66,7 +66,7 @@ test('rule: pr-url', (t) => {
}
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.test('valid', (tt) => {
Expand All @@ -89,7 +89,7 @@ test('rule: pr-url', (t) => {
}
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.test('valid URL containing hyphen', (tt) => {
Expand All @@ -112,7 +112,7 @@ test('rule: pr-url', (t) => {
}
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.test('valid URL with trailing slash', (tt) => {
Expand All @@ -135,7 +135,7 @@ test('rule: pr-url', (t) => {
}
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.end()
Expand Down
4 changes: 2 additions & 2 deletions test/rules/reviewers.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ This is a test`
tt.equal(opts.level, 'fail', 'level')
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.test('skip for release commit', (tt) => {
Expand Down Expand Up @@ -58,7 +58,7 @@ This is a test`
})
}

Rule.validate(context)
Rule.validate(context, { options: {} })
})

t.end()
Expand Down
78 changes: 78 additions & 0 deletions test/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,53 @@ Date: Sat Oct 22 10:22:43 2022 +0200
This reverts commit 01bc8e6fd81314e76c7fb0d09e5310f609e48bee.
`

const str13 = `commit 287d681575bdd2099b0cc892f6a90b0323033101
Author: Stephen Belanger <admin@stephenbelanger.com>
Date: Wed Jul 24 10:00:04 2024 -0700

deps: V8: backport 7857eb34db42

Original commit message:

Reland^2 "Add ContinuationPreservedEmbedderData builtins to extras binding"

This reverts commit cb1277e97a0ed32fd893be9f4e927f6e8b6c566c.

> Original change's description:
> > Add ContinuationPreservedEmbedderData builtins to extras binding
> >
> > Node.js and Deno wish to use CPED for AsyncLocalStorage and APM, which
> > needs a high performance implementation. These builtins allow JavaScript
> > to handle CPED performantly.
> >
> > Change-Id: I7577be80818524baa52791dfce57d442d7c0c933
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5638129
> > Commit-Queue: snek <snek@chromium.org>
> > Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
> > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> > Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#94607}
>
> Change-Id: Ief390f0b99891c8de83b4c794180440f91cbaf1f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5649024
> Auto-Submit: Shu-yu Guo <syg@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#94608}

Change-Id: I4943071ffe192084e83bfe3113cfe9c92ef31465
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5677045
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: snek <snek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#94866}

Refs: v8/v8@7857eb3
`

test('Validator - misc', (t) => {
const v = new Validator()

Expand Down Expand Up @@ -453,5 +500,36 @@ test('Validator - real commits', (t) => {
})
})

t.test('validate no metadata should report commits for having metadata', (tt) => {
const v = new Validator({
'validate-no-metadata': true
})
v.lint(str)
v.on('commit', (data) => {
const msgs = data.messages
const filtered = msgs.filter((item) => {
return item.level === 'fail'
})
tt.equal(filtered.length, 2, 'messages.length')
tt.end()
})
})

t.test('validate no metadata should not report commits without metadata', (tt) => {
const v = new Validator({
'validate-no-metadata': true
})
tt.plan(2)
v.lint(str5)
v.lint(str13)
v.on('commit', (data) => {
const msgs = data.messages
const filtered = msgs.filter((item) => {
return item.level === 'fail'
})
tt.equal(filtered.length, 0, 'messages.length')
})
})

t.end()
})