-
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
Bugfix: Fixes clean command #815
Conversation
@@ -51,6 +51,8 @@ class ProjectBuilder { | |||
buildCmd = ':app:bundleRelease'; | |||
} else if (cmd === 'debug') { | |||
buildCmd = ':app:bundleDebug'; | |||
} else { |
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.
What about assigning the cmd
value already to buildCmd
in line 49? (i.e., let buildCmd = cmd;
) This would remove the else block and only overwrite the buildCmd
in the necessary debug and release cases.
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.
Yah I could do that for simplicity. Probably will only get a chance to make the commit tomorrow night.
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.
No worries, take your time
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.
@timbru31 I've made the commit.
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.
👍 LGTM
Platforms affected
Android
Motivation and Context
Fixes the
cordova clean
command, a reprecussion bug from #764Fixes #813
Description
Due to some code style refactoring, it became possible for the
buildCmd
to stayundefined
, when it used to default to the incoming parametercmd
value. This fixesProjectBuilder.getArgs
to fallback tocmd
value if it doesn't enter in anyif
conditions.Testing
npm test
& manually testing.Note that at the time of working on this issue, master currently did not passnpm test
. So I have "all new and existing tests pass" checkbox unchecked and checking that box is blocked by #814Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)