Improve directory name extraction from branch name.#495
Improve directory name extraction from branch name.#495dreid wants to merge 3 commits intodependabot:mainfrom
Conversation
a4a8652 to
66389d7
Compare
|
👋 thanks for this! I'm on paternity leave right now so hesitant to merge this until I return, as I don't want to risk any problems that others would have to cleanup... but when I return in June I plan to take a look at this and hopefully ship it. |
| if (data['updated-dependencies']) { | ||
| return await Promise.all(data['updated-dependencies'].map(async (dependency, index) => { | ||
| const dirname = `/${chunks.slice(2, -1 * (1 + (dependency['dependency-name'].match(/\//g) || []).length)).join(delim) || ''}` | ||
| const dirname = `/${chunks.slice(2, -1 * (delimIsDash ? 2 : 1 + (dependency['dependency-name'].match(/\//g) || []).length)).join('/') || ''}` |
There was a problem hiding this comment.
Hi I'm trying to wrap my head around this change. Would you mind explaining how it works and what featch-metadata was doing before?
Also, I'm surprised that we didn't have to update any existing tests! I guess we didn't have any test cases for separators?
|
There are some merge conflicts that you will need to resolve by rebasing/merging and re-running the build script before I can merge this |
Nishnha
left a comment
There was a problem hiding this comment.
The change lgtm but there are some merge conflicts that will need to be resolved before it can be pulled in
|
Hi @dreid , Conflicting files |
|
I believe these underlying issues were already fixed by a44a9df |
Fixes #493
Fixes #494
This improves directory extraction to handle two previously unhandled cases:
/-included in thedepname-versionportion is properly accounted for when the delimiter is-.I did not address the fact that the existing non-standard separator test uses a separator (
|) that does not appear to be valid according to the dependabot documention.