-
Notifications
You must be signed in to change notification settings - Fork 997
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(cli): config refactor #3533
Conversation
@ptitjes Would you be able to review/test the changes around |
Conflicts: cli/src/android/add.ts cli/src/android/common.ts cli/src/android/doctor.ts cli/src/android/open.ts cli/src/android/update.ts cli/src/common.ts cli/src/config.ts cli/src/cordova.ts cli/src/index.ts cli/src/ios/add.ts cli/src/ios/common.ts cli/src/ios/doctor.ts cli/src/ios/open.ts cli/src/ios/update.ts cli/src/plugin.ts cli/src/tasks/add.ts cli/src/tasks/copy.ts cli/src/tasks/create.ts cli/src/tasks/doctor.ts cli/src/tasks/init.ts cli/src/tasks/list.ts cli/src/tasks/new-plugin.ts cli/src/tasks/open.ts cli/src/tasks/serve.ts cli/src/tasks/sync.ts cli/src/tasks/update.ts cli/src/web/copy.ts cli/test/util.ts
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.
Added a minor comment.
Also, the way for users to configure the android studio path was windowsAndroidStudioPath
and linuxAndroidStudioPath
. The PR seems to remove those config options (so breaking change? should be documented?).
In open.ts it's still mentioned that you should use them, so texts should be updated for the new way of configuring them.
And in capacitor.config.json
the plugins object appears the first one, since it can grow a lot if users add a lot of configurations, the other values would be hard to find, probably better to print the plugins object the last
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
I didn't mean to break the Android Studio path stuff at this time, so I fixed it. But I think we should change it before the final release. Fixed the |
Config
class replaced with a function that returns a readonly objectresolveNode
now just usesrequire.resolve
with most statements changed to resolve to root of packages using each package'spackage.json