-
-
Notifications
You must be signed in to change notification settings - Fork 522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: escape file names for make
step
#2752
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2752 +/- ##
==========================================
+ Coverage 74.06% 74.07% +0.01%
==========================================
Files 77 77
Lines 2356 2357 +1
Branches 436 436
==========================================
+ Hits 1745 1746 +1
Misses 462 462
Partials 149 149
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't appear to have updated the lockfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice if there were tests added for this.
oops this PR was not done. i wrangled proxyquire enough to write the worst test in the world. PTAL. |
@MarshallOfSound I grabbed this code change from Packager, which also uses the exact same I think since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the foo/null
fixture necessary?
I stole it from https://github.com/electron-userland/electron-forge/tree/master/packages/api/core/test/fixture/dummy_app and made changes accordingly. Not sure if it's necessary, but it follows existing conventions from the repo. |
It's probably not necessary. It's likely not necessary in |
f5407a7 passed locally |
e4c6a1f makes the test a bit better now that i understand things a bit better |
Summarize your changes:
Fixes #1745
Stole some code from Packager which sanitizes file names:
Note: the lockfile doesn't change here because we already had this exact version of filenamify as a transitive dep (via packager).