core-validate-commit - npm Package Compare versions

Comparing version 3.10.1 to 3.11.0

lib/rules/fixes-url.js

 'use strict'
 
 const id = 'fixes-url'
+const github = new RegExp('^https://github\.com/\\w+\/\\w+/' +
+  '(issues|pull)/\\d+(#issuecomment-\\d+|#discussion_r\\d+)?$'
+)
 
@@ -24,3 +27,6 @@ module.exports = {
     }
+    // Allow GitHub issues with optional comment.
+    // GitHub pull requests must reference a comment or discussion.
     for (const url of parsed.fixes) {
+      const match = github.exec(url)
       if (url[0] === '#') {
@@ -32,3 +38,3 @@ // See nodejs/node#2aa376914b621018c5784104b82c13e78ee51307
           id: id
-        , message: 'Fixes must be a url, not an issue number.'
+        , message: 'Fixes must be a URL, not an issue number.'
         , string: url
@@ -39,2 +45,24 @@ , line: line
         })
+      } else if (match) {
+        if (match[1] === 'pull' && match[2] === undefined) {
+          const { line, column } = findLineAndColumn(context.body, url)
+          context.report({
+            id: id
+          , message: 'Pull request URL must reference a comment or discussion.'
+          , string: url
+          , line: line
+          , column: column
+          , level: 'fail'
+          })
+        } else {
+          const { line, column } = findLineAndColumn(context.body, url)
+          context.report({
+            id: id
+          , message: 'Valid fixes URL.'
+          , string: url
+          , line: line
+          , column: column
+          , level: 'pass'
+          })
+        }
       } else {
@@ -44,7 +72,7 @@ const { line, column } = findLineAndColumn(context.body, url)
           id: id
-        , message: 'Valid fixes url'
+        , message: 'Fixes must be a GitHub URL.'
         , string: url
         , line: line
         , column: column
-        , level: 'pass'
+        , level: 'fail'
         })
@@ -51,0 +79,0 @@ }

lib/rules/pr-url.js

 'use strict'
 
 const id = 'pr-url'
+const prUrl = /^https:\/\/github\.com\/\w+\/\w+\/pull\/\d+$/
 
@@ -25,29 +26,38 @@ module.exports = {
     }
-    if (context.prUrl) {
-      if (context.prUrl[0] === '#') {
-        // see nodejs/node#7d3a7ea0d7df9b6f11df723dec370f49f4f87e99
-        // for an example
-        var line = -1
-        var column = -1
-        for (var i = 0; i < context.body.length; i++) {
-          const l = context.body[i]
-          if (~l.indexOf('PR-URL') && ~l.indexOf(context.prUrl)) {
-            line = i
-            column = l.indexOf(context.prUrl)
-          }
-        }
-        context.report({
-          id: id
-        , message: 'PR-URL must be a url, not a pr number.'
-        , string: context.prUrl
-        , line: line
-        , column: column
-        , level: 'fail'
-        })
+    var line = -1
+    var column = -1
+    for (var i = 0; i < context.body.length; i++) {
+      const l = context.body[i]
+      if (~l.indexOf('PR-URL') && ~l.indexOf(context.prUrl)) {
+        line = i
+        column = l.indexOf(context.prUrl)
       }
+    }
+    if (context.prUrl[0] === '#') {
+      // see nodejs/node#7d3a7ea0d7df9b6f11df723dec370f49f4f87e99
+      // for an example
+      context.report({
+        id: id
+      , message: 'PR-URL must be a URL, not a pull request number.'
+      , string: context.prUrl
+      , line: line
+      , column: column
+      , level: 'fail'
+      })
+    } else if (!prUrl.test(context.prUrl)) {
+      context.report({
+        id: id
+      , message: 'PR-URL must be a GitHub pull request URL.'
+      , string: context.prUrl
+      , line: line
+      , column: column
+      , level: 'fail'
+      })
     } else {
       context.report({
         id: id
-      , message: 'PR-URL is valid'
+      , message: 'PR-URL is valid.'
       , string: context.prUrl
+      , line: line
+      , column: column
       , level: 'pass'
@@ -54,0 +64,0 @@ })

lib/rules/subsystem.js

@@ -55,2 +55,3 @@ 'use strict'
 , 'querystring'
+, 'quic'
 , 'readline'
@@ -57,0 +58,0 @@ , 'repl'

package.json

 {
   "name": "core-validate-commit",
-  "version": "3.10.1",
+  "version": "3.11.0",
   "description": "Validate the commit message for a particular commit in node core",
@@ -5,0 +5,0 @@ "main": "index.js",

README.md

 # core-validate-commit
 
-[![Build Status](https://travis-ci.org/nodejs/core-validate-commit.svg)](https://travis-ci.org/nodejs/core-validate-commit)
-[![Coverage Status](https://coveralls.io/repos/evanlucas/core-validate-commit/badge.svg?branch=master&service=github)](https://coveralls.io/github/evanlucas/core-validate-commit?branch=master)
+[![Build Status](https://travis-ci.com/nodejs/core-validate-commit.svg?branch=master)](https://travis-ci.com/nodejs/core-validate-commit)
 
@@ -6,0 +5,0 @@ Validate the commit message for a particular commit in node core

test/rules/fixes-url.js

@@ -7,19 +7,27 @@ 'use strict'
 const Validator = require('../../')
-const INVALID_FIXES_URL = 'Fixes must be a url, not an issue number.'
 
+const INVALID_PRURL = 'Pull request URL must reference a comment or discussion.'
+const NOT_AN_ISSUE_NUMBER = 'Fixes must be a URL, not an issue number.'
+const NOT_A_GITHUB_URL = 'Fixes must be a GitHub URL.'
+const VALID_FIXES_URL = 'Valid fixes URL.'
+
+const makeCommit = (msg) => {
+  return new Commit({
+    sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea'
+  , author: {
+      name: 'Evan Lucas'
+    , email: '[e-mail]'
+    , date: '2016-04-12T19:42:23Z'
+    }
+  , message: msg
+  }, new Validator())
+}
+
 test('rule: fixes-url', (t) => {
-  t.test('invalid', (tt) => {
+  t.test('issue number', (tt) => {
     tt.plan(7)
-    const v = new Validator()
-    const context = new Commit({
-      sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea'
-    , author: {
-        name: 'Evan Lucas'
-      , email: '[e-mail]'
-      , date: '2016-04-12T19:42:23Z'
-      }
-    , message: `test: fix something
+    const context = makeCommit(`test: fix something
 
 Fixes: #1234`
-    }, v)
+    )
 
@@ -29,3 +37,3 @@ context.report = (opts) => {
       tt.equal(opts.id, 'fixes-url', 'id')
-      tt.equal(opts.message, INVALID_FIXES_URL, 'message')
+      tt.equal(opts.message, NOT_AN_ISSUE_NUMBER, 'message')
       tt.equal(opts.string, '#1234', 'string')
@@ -40,3 +48,128 @@ tt.equal(opts.line, 1, 'line')
 
+  t.test('GitHub issue URL', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/issues/1234'
+    const context = makeCommit(`test: fix something
+
+Fixes: ${url}`
+    )
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.equal(opts.id, 'fixes-url', 'id')
+      tt.equal(opts.message, VALID_FIXES_URL, 'message')
+      tt.equal(opts.string, url, 'string')
+      tt.equal(opts.line, 1, 'line')
+      tt.equal(opts.column, 7, 'column')
+      tt.equal(opts.level, 'pass', 'level')
+    }
+
+    Rule.validate(context)
+  })
+
+  t.test('GitHub issue URL with comment', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/issues/1234#issuecomment-1234'
+    const context = makeCommit(`test: fix something
+
+Fixes: ${url}`
+    )
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.equal(opts.id, 'fixes-url', 'id')
+      tt.equal(opts.message, VALID_FIXES_URL, 'message')
+      tt.equal(opts.string, url, 'string')
+      tt.equal(opts.line, 1, 'line')
+      tt.equal(opts.column, 7, 'column')
+      tt.equal(opts.level, 'pass', 'level')
+    }
+
+    Rule.validate(context)
+  })
+
+  t.test('GitHub PR URL', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/pull/1234'
+    const context = makeCommit(`test: fix something
+
+Fixes: ${url}`
+    )
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.equal(opts.id, 'fixes-url', 'id')
+      tt.equal(opts.message, INVALID_PRURL, 'message')
+      tt.equal(opts.string, url, 'string')
+      tt.equal(opts.line, 1, 'line')
+      tt.equal(opts.column, 7, 'column')
+      tt.equal(opts.level, 'fail', 'level')
+    }
+
+    Rule.validate(context)
+  })
+
+  t.test('GitHub PR URL with comment', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/pull/1234#issuecomment-1234'
+    const context = makeCommit(`test: fix something
+
+Fixes: ${url}`
+    )
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.equal(opts.id, 'fixes-url', 'id')
+      tt.equal(opts.message, VALID_FIXES_URL, 'message')
+      tt.equal(opts.string, url, 'string')
+      tt.equal(opts.line, 1, 'line')
+      tt.equal(opts.column, 7, 'column')
+      tt.equal(opts.level, 'pass', 'level')
+    }
+
+    Rule.validate(context)
+  })
+
+  t.test('GitHub PR URL with discussion comment', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/pull/1234#discussion_r1234'
+    const context = makeCommit(`test: fix something
+
+Fixes: ${url}`
+    )
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.equal(opts.id, 'fixes-url', 'id')
+      tt.equal(opts.message, VALID_FIXES_URL, 'message')
+      tt.equal(opts.string, url, 'string')
+      tt.equal(opts.line, 1, 'line')
+      tt.equal(opts.column, 7, 'column')
+      tt.equal(opts.level, 'pass', 'level')
+    }
+
+    Rule.validate(context)
+  })
+
+  t.test('non-GitHub URL', (tt) => {
+    tt.plan(7)
+    const context = makeCommit(`test: fix something
+
+Fixes: https://nodejs.org`
+    )
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.equal(opts.id, 'fixes-url', 'id')
+      tt.equal(opts.message, NOT_A_GITHUB_URL, 'message')
+      tt.equal(opts.string, 'https://nodejs.org', 'string')
+      tt.equal(opts.line, 1, 'line')
+      tt.equal(opts.column, 7, 'column')
+      tt.equal(opts.level, 'fail', 'level')
+    }
+
+    Rule.validate(context)
+  })
+
   t.end()
 })

test/rules/pr-url.js

@@ -6,3 +6,5 @@ 'use strict'
 const MISSING_PR_URL = 'Commit must have a PR-URL.'
-const INVALID_PR_URL = 'PR-URL must be a url, not a pr number.'
+const INVALID_PR_URL = 'PR-URL must be a GitHub pull request URL.'
+const NUMERIC_PR_URL = 'PR-URL must be a URL, not a pull request number.'
+const VALID_PR_URL = 'PR-URL is valid.'
 
@@ -28,3 +30,4 @@ test('rule: pr-url', (t) => {
 
-  t.test('invalid', (tt) => {
+
+  t.test('invalid numeric', (tt) => {
     tt.plan(7)
@@ -40,3 +43,3 @@ const context = {
         tt.equal(opts.id, 'pr-url', 'id')
-        tt.equal(opts.message, INVALID_PR_URL, 'message')
+        tt.equal(opts.message, NUMERIC_PR_URL, 'message')
         tt.equal(opts.string, '#1234', 'string')
@@ -52,3 +55,49 @@ tt.equal(opts.line, 1, 'line')
 
+  t.test('invalid', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/issues/1234'
+    const context = {
+      prUrl: url
+    , body: [
+        ''
+      , `PR-URL: ${url}`
+      ]
+    , report: (opts) => {
+        tt.pass('called report')
+        tt.equal(opts.id, 'pr-url', 'id')
+        tt.equal(opts.message, INVALID_PR_URL, 'message')
+        tt.equal(opts.string, url, 'string')
+        tt.equal(opts.line, 1, 'line')
+        tt.equal(opts.column, 8, 'column')
+        tt.equal(opts.level, 'fail', 'level')
+      }
+    }
+
+    Rule.validate(context)
+  })
+
+  t.test('valid', (tt) => {
+    tt.plan(7)
+    const url = 'https://github.com/nodejs/node/pull/1234'
+    const context = {
+      prUrl: url
+    , body: [
+        ''
+      , `PR-URL: ${url}`
+      ]
+    , report: (opts) => {
+        tt.pass('called report')
+        tt.equal(opts.id, 'pr-url', 'id')
+        tt.equal(opts.message, VALID_PR_URL, 'message')
+        tt.equal(opts.string, url, 'string')
+        tt.equal(opts.line, 1, 'line')
+        tt.equal(opts.column, 8, 'column')
+        tt.equal(opts.level, 'pass', 'level')
+      }
+    }
+
+    Rule.validate(context)
+  })
+
   t.end()
 })

test/rules/reviewers.js

@@ -38,3 +38,31 @@ 'use strict'
 
+  t.test('skip for release commit', (tt) => {
+    tt.plan(2)
+    const v = new Validator()
+    const context = new Commit({
+      sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea'
+    , author: {
+        name: 'Evan Lucas'
+      , email: '[e-mail]'
+      , date: '2016-04-12T19:42:23Z'
+      }
+    , message: `2016-04-12, Version x.y.z
+
+This is a test`
+    }, v)
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.strictSame(opts, {
+        id: 'reviewers'
+      , message: 'skipping reviewers for release commit'
+      , string: ''
+      , level: 'skip'
+      })
+    }
+
+    Rule.validate(context)
+  })
+
   t.end()
 })

test/rules/subsystem.js

@@ -36,2 +36,30 @@ 'use strict'
 
+  t.test('skip for release commit', (tt) => {
+    tt.plan(2)
+
+    const v = new Validator()
+    const context = new Commit({
+      sha: 'e7c077c610afa371430180fbd447bfef60ebc5ea'
+    , author: {
+        name: 'Evan Lucas'
+      , email: '[e-mail]'
+      , date: '2016-04-12T19:42:23Z'
+      }
+    , message: '2016-04-12, Version x.y.z'
+    }, v)
+
+    context.report = (opts) => {
+      tt.pass('called report')
+      tt.strictSame(opts, {
+        id: 'subsystem'
+      , message: 'Release commits do not have subsystems'
+      , string: ''
+      , level: 'skip'
+      })
+      tt.end()
+    }
+
+    Rule.validate(context, {options: {subsystems: Rule.defaults.subsystems}})
+  })
+
   t.test('valid', (tt) => {
@@ -48,3 +76,3 @@ tt.plan(2)
       }
-    , message: 'worker: come on, fhqwhgads'
+    , message: 'quic: come on, fhqwhgads'
     }, v)
@@ -57,3 +85,3 @@
       , message: 'valid subsystems'
-      , string: 'worker'
+      , string: 'quic'
       , level: 'pass'
@@ -66,4 +94,3 @@ })
   })
-
   t.end()
 })

No alert changes

Improved metrics

Total package byte prevSize

66690

increased by12.32%

Lines of code

2177

increased by12.27%

Worsened metrics

Number of lines in readme file

49

decreased by-2%

No dependency changes