[build-script]: use / as the path separator for glob.sync#15140
[build-script]: use / as the path separator for glob.sync#15140joshuaellis merged 2 commits intomainfrom
Conversation
joshuaellis
added
source: tooling
Codecov ReportBase: 59.88% // Head: 59.88% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #15140 +/- ##
==========================================
- Coverage 59.88% 59.88% -0.01%
==========================================
Files 1348 1348
Lines 32800 32803 +3
Branches 6255 6256 +1
==========================================
+ Hits 19642 19643 +1
- Misses 11303 11305 +2
Partials 1855 1855
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
innerdvations
left a comment
innerdvations
left a comment
There was a problem hiding this comment.
Looks good to me!
I have one question I'm not sure about and can't test myself right now. Does process.platform under WSL report "win32" or does it report the subsystem's platform? I don't think it should matter here, but I just want to be sure we don't accidentally break something on WSL to fix vanilla Windows.
| if (process.platform === 'win32') { | ||
| corePath = corePath.split(sep).join(posix.sep); | ||
| pluginsPath = pluginsPath.split(sep).join(posix.sep); | ||
| } |
There was a problem hiding this comment.
Could you add a test for this branch (by mocking process.platform), because I imagine this will easily be forgotten about ...?
There was a problem hiding this comment.
I can try, have you checked the tests for this? They're a bit odd. They rely on actually using path and we only run tests in a macOS env, so I'm not sure the best way? Any ideas?
There was a problem hiding this comment.
Suppose I could mock path for the test?
There was a problem hiding this comment.
I haven't looked, but can try to suggest something if you want next week? Shouldn't prevent you from merging this PR ...
|
@joshuaellis Was this tested on a Linux OS like Ubuntu 22.04? I ask because I am having trouble building my Strapi workspace with My bug report: #15311 |
4 participants
What does it do?
Why is it needed?
globdoes not support the windows separator as documented here and you can see the recommended fix here. This seemed to be a big thing for v8.How to test it?
Related issue(s)/PR(s)