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.

Sign up for GitHub

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

Jump to bottom

lib: skip fixup! and squash! commits #33

Closed
wants to merge 5 commits into from
Closed

lib: skip fixup! and squash! commits #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
79f946e
lib: skip fixup! and squash! commits
richardlau Nov 9, 2018
d07d944
squash! lib: skip fixup! and squash! commits
richardlau Nov 10, 2018
25e5398
fixup! lib: skip fixup! and squash! commits
richardlau Nov 10, 2018
47f98de
fixup! lib: skip fixup! and squash! commits
richardlau Nov 10, 2018
229ba0c
fixup! lib: skip fixup! and squash! commits
richardlau Nov 10, 2018
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
2 changes: 2 additions & 0 deletions bin/cmd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ const knownOpts = { help: Boolean
, tap: Boolean
, out: path
, list: Boolean
, 'check-autosquashable': Boolean
}
const shortHand = { h: ['--help']
, v: ['--version']
, V: ['--validate-metadata']
, t: ['--tap']
, o: ['--out']
, l: ['--list']
, a: ['--check-autosquashable']
}

const parsed = nopt(knownOpts, shortHand)
Expand Down
1 change: 1 addition & 0 deletions bin/usage.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ core-validate-commit - Validate the commit message for a particular commit in no
-V, --validate-metadata validate PR-URL and reviewers (on by default)
-t, --tap output in tap format
-l, --list list rules and their descriptions
-a, --check-autosquashable check autosquashable commits (on by default)

examples:
Validate a single sha:
Expand Down
13 changes: 10 additions & 3 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = class ValidateCommit extends EE {

this.opts = Object.assign({
'validate-metadata': true
, 'check-autosquashable': true
}, options)

this.messages = new Map()
Expand Down Expand Up @@ -47,9 +48,15 @@ module.exports = class ValidateCommit extends EE {
}
} else {
const commit = new Parser(str, this)
for (const rule of this.rules.values()) {
if (rule.disabled) continue
rule.validate(commit)
if (!this.opts['check-autosquashable'] &&
Copy link
Member

@joyeecheung joyeecheung Nov 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I am a bit confused..shouldn't it be named skip-autosquashable (off by default)? Because even when this is turned off via a --no switch we are still going to apply that rule to the commit for the warning

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the option as conceptual (i.e. a signal of intent) and the rule an implementation detail. I followed the existing validate-metadata option (which is also on by default and is not, for example, skip-metadata).

Initially I didn't have a rule at all and implemented the skip logic directly here. Maybe it shouldn't be a rule at all (for example it doesn't really have pass/fail states). It's sort of a meta "if the commit looks like this then don't apply the usual rules but apply this special one to emit a warning instead".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@richardlau ACK. Somehow it feels more natural to me to say the intent is "skip autosquashability checks", and the rules are "check if the commits are autosqushable" i.e. I think it would be more intuitive if we switch the two names, the option being skip-autosquash-checks and the rule being check-autosquashable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..and, it may more natural if we split the "warn" checks and the "fail" checks into two rules, if the sole purpose of the warning rules is to get some warnings displayed..so we can keep the existing skip-autosquashable.js but strip the "fail" checks into a check-autosquashable.js

(commit.title.startsWith('fixup!') ||
commit.title.startsWith('squash!'))) {
this.rules.get('skip-autosquashable').validate(commit)
} else {
for (const rule of this.rules.values()) {
if (rule.disabled) continue
rule.validate(commit)
}
}

setImmediate(() => {
Expand Down
44 changes: 44 additions & 0 deletions lib/rules/skip-autosquashable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict'

const id = 'skip-autosquashable'

module.exports = {
id: id
, meta: {
description: 'skip autosquashable fixup! and squash! commits'
, recommended: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..so that the recommended and disabled here should be both false by default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't work out what recommended is for (or find code that does anything with it).

}
, disabled: true
, defaults: { }
, options: { }
, validate: (context, rule) => {
if (context.title.startsWith('fixup!')) {
context.report({
id: id
, message: 'Skipping fixup! commit.'
, string: context.title
, level: 'warn'
})
return
}

if (context.title.startsWith('squash!')) {
context.report({
id: id
, message: 'Skipping squash! commit.'
, string: context.title
, level: 'warn'
})
return
}

context.report({
id: id
, message: 'Commit is neither fixup! nor squash!.'
, string: context.title
, line: 0
, column: 0
, level: 'fail'
})
}
}
239 changes: 239 additions & 0 deletions test/fixtures/autosquashable-pr.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
[
{
"sha": "79f946e88e79f5063bbe775380a418bd8ab0b881",
"node_id": "MDY6Q29tbWl0MTU1ODE5NzI2Ojc5Zjk0NmU4OGU3OWY1MDYzYmJlNzc1MzgwYTQxOGJkOGFiMGI4ODE=",
"commit": {
"author": {
"name": "Richard Lau",
"email": "riclau@uk.ibm.com",
"date": "2018-11-09T16:24:33Z"
},
"committer": {
"name": "Richard Lau",
"email": "riclau@uk.ibm.com",
"date": "2018-11-09T18:19:29Z"
},
"message": "lib: skip fixup! and squash! commits",
"tree": {
"sha": "0117c48bf2beff44676b0fa8c4e652860d67e833",
"url": "https://api.github.com/repos/nodejs/core-validate-commit/git/trees/0117c48bf2beff44676b0fa8c4e652860d67e833"
},
"url": "https://api.github.com/repos/nodejs/core-validate-commit/git/commits/79f946e88e79f5063bbe775380a418bd8ab0b881",
"comment_count": 0,
"verification": {
"verified": false,
"reason": "unsigned",
"signature": null,
"payload": null
}
},
"url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/79f946e88e79f5063bbe775380a418bd8ab0b881",
"html_url": "https://github.com/nodejs/core-validate-commit/commit/79f946e88e79f5063bbe775380a418bd8ab0b881",
"comments_url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/79f946e88e79f5063bbe775380a418bd8ab0b881/comments",
"author": {
"login": "richardlau",
"id": 5445507,
"node_id": "MDQ6VXNlcjU0NDU1MDc=",
"avatar_url": "https://avatars0.githubusercontent.com/u/5445507?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/richardlau",
"html_url": "https://github.com/richardlau",
"followers_url": "https://api.github.com/users/richardlau/followers",
"following_url": "https://api.github.com/users/richardlau/following{/other_user}",
"gists_url": "https://api.github.com/users/richardlau/gists{/gist_id}",
"starred_url": "https://api.github.com/users/richardlau/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/richardlau/subscriptions",
"organizations_url": "https://api.github.com/users/richardlau/orgs",
"repos_url": "https://api.github.com/users/richardlau/repos",
"events_url": "https://api.github.com/users/richardlau/events{/privacy}",
"received_events_url": "https://api.github.com/users/richardlau/received_events",
"type": "User",
"site_admin": false
},
"committer": {
"login": "richardlau",
"id": 5445507,
"node_id": "MDQ6VXNlcjU0NDU1MDc=",
"avatar_url": "https://avatars0.githubusercontent.com/u/5445507?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/richardlau",
"html_url": "https://github.com/richardlau",
"followers_url": "https://api.github.com/users/richardlau/followers",
"following_url": "https://api.github.com/users/richardlau/following{/other_user}",
"gists_url": "https://api.github.com/users/richardlau/gists{/gist_id}",
"starred_url": "https://api.github.com/users/richardlau/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/richardlau/subscriptions",
"organizations_url": "https://api.github.com/users/richardlau/orgs",
"repos_url": "https://api.github.com/users/richardlau/repos",
"events_url": "https://api.github.com/users/richardlau/events{/privacy}",
"received_events_url": "https://api.github.com/users/richardlau/received_events",
"type": "User",
"site_admin": false
},
"parents": [
{
"sha": "4e64157b6a87267fd4b9f0598159eeb968e04e21",
"url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/4e64157b6a87267fd4b9f0598159eeb968e04e21",
"html_url": "https://github.com/nodejs/core-validate-commit/commit/4e64157b6a87267fd4b9f0598159eeb968e04e21"
}
]
},
{
"sha": "d07d94439be43a65d6e57ec2ee1d8c93676e8d55",
"node_id": "MDY6Q29tbWl0MTU1ODE5NzI2OmQwN2Q5NDQzOWJlNDNhNjVkNmU1N2VjMmVlMWQ4YzkzNjc2ZThkNTU=",
"commit": {
"author": {
"name": "Richard Lau",
"email": "riclau@uk.ibm.com",
"date": "2018-11-10T00:44:48Z"
},
"committer": {
"name": "Richard Lau",
"email": "riclau@uk.ibm.com",
"date": "2018-11-10T00:44:48Z"
},
"message": "squash! lib: skip fixup! and squash! commits",
"tree": {
"sha": "b37fb650a27feaddb950d153a7085fec2347ec7d",
"url": "https://api.github.com/repos/nodejs/core-validate-commit/git/trees/b37fb650a27feaddb950d153a7085fec2347ec7d"
},
"url": "https://api.github.com/repos/nodejs/core-validate-commit/git/commits/d07d94439be43a65d6e57ec2ee1d8c93676e8d55",
"comment_count": 0,
"verification": {
"verified": false,
"reason": "unsigned",
"signature": null,
"payload": null
}
},
"url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/d07d94439be43a65d6e57ec2ee1d8c93676e8d55",
"html_url": "https://github.com/nodejs/core-validate-commit/commit/d07d94439be43a65d6e57ec2ee1d8c93676e8d55",
"comments_url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/d07d94439be43a65d6e57ec2ee1d8c93676e8d55/comments",
"author": {
"login": "richardlau",
"id": 5445507,
"node_id": "MDQ6VXNlcjU0NDU1MDc=",
"avatar_url": "https://avatars0.githubusercontent.com/u/5445507?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/richardlau",
"html_url": "https://github.com/richardlau",
"followers_url": "https://api.github.com/users/richardlau/followers",
"following_url": "https://api.github.com/users/richardlau/following{/other_user}",
"gists_url": "https://api.github.com/users/richardlau/gists{/gist_id}",
"starred_url": "https://api.github.com/users/richardlau/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/richardlau/subscriptions",
"organizations_url": "https://api.github.com/users/richardlau/orgs",
"repos_url": "https://api.github.com/users/richardlau/repos",
"events_url": "https://api.github.com/users/richardlau/events{/privacy}",
"received_events_url": "https://api.github.com/users/richardlau/received_events",
"type": "User",
"site_admin": false
},
"committer": {
"login": "richardlau",
"id": 5445507,
"node_id": "MDQ6VXNlcjU0NDU1MDc=",
"avatar_url": "https://avatars0.githubusercontent.com/u/5445507?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/richardlau",
"html_url": "https://github.com/richardlau",
"followers_url": "https://api.github.com/users/richardlau/followers",
"following_url": "https://api.github.com/users/richardlau/following{/other_user}",
"gists_url": "https://api.github.com/users/richardlau/gists{/gist_id}",
"starred_url": "https://api.github.com/users/richardlau/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/richardlau/subscriptions",
"organizations_url": "https://api.github.com/users/richardlau/orgs",
"repos_url": "https://api.github.com/users/richardlau/repos",
"events_url": "https://api.github.com/users/richardlau/events{/privacy}",
"received_events_url": "https://api.github.com/users/richardlau/received_events",
"type": "User",
"site_admin": false
},
"parents": [
{
"sha": "79f946e88e79f5063bbe775380a418bd8ab0b881",
"url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/79f946e88e79f5063bbe775380a418bd8ab0b881",
"html_url": "https://github.com/nodejs/core-validate-commit/commit/79f946e88e79f5063bbe775380a418bd8ab0b881"
}
]
},
{
"sha": "25e5398728b59ab0fdf141d1676a68969204c60a",
"node_id": "MDY6Q29tbWl0MTU1ODE5NzI2OjI1ZTUzOTg3MjhiNTlhYjBmZGYxNDFkMTY3NmE2ODk2OTIwNGM2MGE=",
"commit": {
"author": {
"name": "Richard Lau",
"email": "riclau@uk.ibm.com",
"date": "2018-11-10T01:04:41Z"
},
"committer": {
"name": "Richard Lau",
"email": "riclau@uk.ibm.com",
"date": "2018-11-10T01:04:41Z"
},
"message": "fixup! lib: skip fixup! and squash! commits",
"tree": {
"sha": "c4f1faf780d28fce67c7d1ec0a7e0617f43bc79a",
"url": "https://api.github.com/repos/nodejs/core-validate-commit/git/trees/c4f1faf780d28fce67c7d1ec0a7e0617f43bc79a"
},
"url": "https://api.github.com/repos/nodejs/core-validate-commit/git/commits/25e5398728b59ab0fdf141d1676a68969204c60a",
"comment_count": 0,
"verification": {
"verified": false,
"reason": "unsigned",
"signature": null,
"payload": null
}
},
"url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/25e5398728b59ab0fdf141d1676a68969204c60a",
"html_url": "https://github.com/nodejs/core-validate-commit/commit/25e5398728b59ab0fdf141d1676a68969204c60a",
"comments_url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/25e5398728b59ab0fdf141d1676a68969204c60a/comments",
"author": {
"login": "richardlau",
"id": 5445507,
"node_id": "MDQ6VXNlcjU0NDU1MDc=",
"avatar_url": "https://avatars0.githubusercontent.com/u/5445507?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/richardlau",
"html_url": "https://github.com/richardlau",
"followers_url": "https://api.github.com/users/richardlau/followers",
"following_url": "https://api.github.com/users/richardlau/following{/other_user}",
"gists_url": "https://api.github.com/users/richardlau/gists{/gist_id}",
"starred_url": "https://api.github.com/users/richardlau/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/richardlau/subscriptions",
"organizations_url": "https://api.github.com/users/richardlau/orgs",
"repos_url": "https://api.github.com/users/richardlau/repos",
"events_url": "https://api.github.com/users/richardlau/events{/privacy}",
"received_events_url": "https://api.github.com/users/richardlau/received_events",
"type": "User",
"site_admin": false
},
"committer": {
"login": "richardlau",
"id": 5445507,
"node_id": "MDQ6VXNlcjU0NDU1MDc=",
"avatar_url": "https://avatars0.githubusercontent.com/u/5445507?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/richardlau",
"html_url": "https://github.com/richardlau",
"followers_url": "https://api.github.com/users/richardlau/followers",
"following_url": "https://api.github.com/users/richardlau/following{/other_user}",
"gists_url": "https://api.github.com/users/richardlau/gists{/gist_id}",
"starred_url": "https://api.github.com/users/richardlau/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/richardlau/subscriptions",
"organizations_url": "https://api.github.com/users/richardlau/orgs",
"repos_url": "https://api.github.com/users/richardlau/repos",
"events_url": "https://api.github.com/users/richardlau/events{/privacy}",
"received_events_url": "https://api.github.com/users/richardlau/received_events",
"type": "User",
"site_admin": false
},
"parents": [
{
"sha": "d07d94439be43a65d6e57ec2ee1d8c93676e8d55",
"url": "https://api.github.com/repos/nodejs/core-validate-commit/commits/d07d94439be43a65d6e57ec2ee1d8c93676e8d55",
"html_url": "https://github.com/nodejs/core-validate-commit/commit/d07d94439be43a65d6e57ec2ee1d8c93676e8d55"
}
]
}
]
Loading