fix: potential non-url Fixes being added to metadata #485
Conversation
codebytere
changed the title
Codecov Report
@@ Coverage Diff @@
## master #485 +/- ##
==========================================
+ Coverage 82.59% 82.60% +0.01%
==========================================
Files 34 34
Lines 1660 1661 +1
==========================================
+ Hits 1371 1372 +1
Misses 289 289
Continue to review full report at Codecov.
|
|
Would it still allow URLs? |
|
From what I can tell this would make links like |
|
Missed a spot in the regex - should be fixed up now! |
codebytere
force-pushed
the
improve-fixes-regex
branch
from
39ae60d
to
44ad7cf
Compare
|
The goal lgtm, would need more time to review the regex (if others think the regex is fine I'm good with it too!) |
|
Tests would be good. Particularly one showing an example of a non-url Fixes being ignored and one showing a Fixes with a full URL is accepted. |
| @@ -5,6 +5,8 @@ const FIX_RE = /(Close[ds]?|Fix(e[ds])?|Resolve[sd]?)\s*:\s*(\S+)/i; | |||
| const REFS_RE = /Refs?\s*:\s*(\S+)/mgi; | |||
| const REF_RE = /Refs?\s*:\s*(\S+)/i; | |||
| const PR_RE = /PR-URL\s*:\s*(\S+)/i; | |||
| const LINK_RE = /(#?\d+)|(\S+\/\S+(\/#?|\/?)\d+)/; | |||
There was a problem hiding this comment.
This matches just numbers (first clause), is that right as I don't think it's a valid GitHub Fixes value?
Also, this allows for something like some-text.../12345.
Could I suggest something like
^(#\d+|[^\s/]+\/[^\s/]+#\d+|https?:\/\/github\.com\/[^\s/]+\/[^\s/]+\/(issue|pull)\/\d+)$
^ ^ ^
handle | handle | handle http(s)://github.com/<any>/<any>/issue
same-repo | GitHub | or pull/issue pr number
shorthand | shorthand |
The [^\s/] can be simplified to \w if we don't care about unicode names.
testing link https://regex101.com/r/JCwzau/1, there are a few unit-tests as well
There was a problem hiding this comment.
@lundibundi I do agree it could be more robust - i'm happy to swap to this one :D
|
This PR is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made. |
Closes #233.
Filters out anything from being added to
Fixesmetadata that does not look e.g like:Fixes #16
Fixes nodejs/build#1961
Fixes https://github.com/nodejs/node/issue/12345