Add build type option to release cordova command #409
Conversation
…fferent platforms more clear Due to multiple requests in discord chanel, microsoft/react-native-code-push#723, microsoft/react-native-code-push#717
Also fixes issue microsoft#392
|
@max-mironov, |
cli/README.md
Outdated
| @@ -580,6 +580,7 @@ code-push release-cordova <appName> <platform> | |||
| [--targetBinaryVersion <targetBinaryVersion>] | |||
| [--rollout <rolloutPercentage>] | |||
| [--build] | |||
| [--isReleaseBuildType] | |||
There was a problem hiding this comment.
Both the --build and --isReleaseBuildType flags need to be alphabetically ordered above
cli/README.md
Outdated
| @@ -643,6 +644,10 @@ This is the same parameter as the one described in the [above section](#disabled | |||
|
|
|||
| Specifies whether you want to run `cordova build` instead of `cordova prepare` (which is the default behavior), when generating your updated web assets. This is valuable if your project includes before and/or after build hooks (e.g. to transpile TypeScript), and therefore, having CodePush simply run `cordova prepare` isn't sufficient to create and release an update. If left unspecified, this defaults to `false`. | |||
|
|
|||
| #### IsReleaseBuildType parameter | |||
There was a problem hiding this comment.
Both the Build and IsReleaseBuildType sections need to be alphabetically ordered above
| @@ -1175,7 +1175,9 @@ export var releaseCordova = (command: cli.IReleaseCordovaCommand): Promise<void> | |||
| throw new Error("Platform must be either \"ios\" or \"android\"."); | |||
| } | |||
|
|
|||
| var cordovaCommand: string = command.build ? "build" : "prepare"; | |||
| var cordovaCommand: string = command.build ? | |||
| (command.isReleaseBuildType ? "build --release" : "build") : | |||
There was a problem hiding this comment.
We normally name the flags that we pass through to the underlying react-native or cordova CLI's the same as they would be in the original CLI. This generally makes them more discoverable/consistent for people who already have been using the underlying CLI. I understand why it might have made sense to name it otherwise, but with all things considered do you think it makes sense to just call the flag release instead of isReleaseBuildType? (I'll leave this up to you).
There was a problem hiding this comment.
@Silhouettes thank you for your feedback. I thought initially about this and I was a little bit confused about naming it release because it would be semantically closer to something related to our code-push release command which actually perform the release of our updates to code push server and has nothing in common with release build flag. So I think naming this flag as release may lead to some confusion. If you don't mind I'd prefer to avoid 'release' name here in this particular case.
There was a problem hiding this comment.
Okay sure - just wanted to make sure that was considered, but sounds like you have :)
See #392 for details