-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
refactor: remove shelljs dependency #842
Conversation
3d9778d
to
d96074f
Compare
04fafc4
to
60502f2
Compare
Codecov Report
@@ Coverage Diff @@
## master #842 +/- ##
==========================================
- Coverage 66.27% 65.94% -0.33%
==========================================
Files 19 21 +2
Lines 1853 1850 -3
==========================================
- Hits 1228 1220 -8
- Misses 625 630 +5
Continue to review full report at Codecov.
|
60502f2
to
c327a23
Compare
c327a23
to
afa1741
Compare
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.
I added some code refactor, cleanup, and removed additional references to shelljs that was missed.
I also ran manual tests on macOS and Windows. Tests covers:
cordova platform add
cordova platform
cordova requirements
cordova prepare android
cordova build android
cordova build android
(w/ keystore)cordova run android
(not tested on Windows)cordova clean android
cordova platform rm android
Erisu finished this for me and I've tested it on linux. npm test runs fine, and did some manual testing running |
The pattern contained an additional plus that slipped in during the refactoring done in apache#842. See [the diff][1] for details. [1]: apache@09e8248#diff-26c51bfaa44eff1e46fd61ec3225ec13L640-R650
The pattern contained an additional plus that slipped in during the refactoring done in #842. See [the diff][1] for details. [1]: 09e8248#diff-26c51bfaa44eff1e46fd61ec3225ec13L640-R650
… Windows This changes the implementation to be closer to what it was before apache#842 with everything being in one place.
…ows (#1102) * test(check_reqs): test default Java location detection on Windows * refactor(check_reqs): use glob for default Java location detection on Windows This changes the implementation to be closer to what it was before #842 with everything being in one place. * fix: remove always-taken if statement * feat: take both Program Files variants from env * refactor(check_reqs): cosmetic changes
Platforms affected
Android
Motivation and Context
Replaces
shelljs
with favour for nativefs
APIs, or apis fromfs-extra
Fixes #781
Description
All shelljs code has been removed and replaced. In some cases,
which
package was used as a replacement.Testing
npm test
passes. Manual testing has been done on linux by usingcordova platform add <my git fork>
Windows has not been tested. Test are pending.
Mac has has also not been tested. I don't have easy access to a mac machine, so assistance here would help me.
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)