Split regex and extend tests #18
Conversation
| 'foo/...#123', | ||
| '#123', | ||
| '#999', |
There was a problem hiding this comment.
Side note: The tests are really hard to parse when one is missing, the whole "diff" breaks and it's not really clear what's - and what's +.
This is why I changes a couple to avoid the repetitive 123
I'll look for an alternative later maybe.
Edit: done: #19
index.js
Outdated
| export default function issueRegex() { | ||
| return /(?<!\w)(?:(?<organization>[a-z\d](?:[a-z\d-]{0,37}[a-z\d])?)\/(?<repository>[\w.-]{1,100}))?(?<!(?:\/\.{1,2}))#(?<issueNumber>[1-9]\d{0,9})\b/gi; | ||
| return new RegExp( | ||
| `${initialDelimiter}(?:${organization}\\/${repository})?${reservedRepository}#${issueNumber}\\b`, |
There was a problem hiding this comment.
Nitpick, but this could be a top-level string variable.
|
This looks fine to me. |
|
By the way I may have found some bugs while reworking the tests, so if you do your Node 18 / dependency update commit I’ll follow up with the test updates. #16 would have to wait. |
Are we planning any other breaking changes? If not, I don't see the strong need to do a major update just for Node.js 18. |
|
Just this: https://github.com/xojs/xo/releases/tag/v0.57.0 But the solution is to stay on XO 0.56 |
Splitting the regex makes it much easier to edit.
If this isn't well-liked, I propose one of: