Update option step and create new option for select build system with… #53
Conversation
manuelabarca
force-pushed
the
update-build-system
branch
from
d526552
to
5e67228
Compare
manuelabarca
force-pushed
the
update-build-system
branch
from
5e67228
to
40a8fd6
Compare
main.go
Outdated
| legacyQuery := "--buildFlag='-UseModernBuildSystem=0'" | ||
| builder.SetCustomOptions(legacyQuery) | ||
| } else if configs.BuildSystem == "modern" { | ||
| modernQuery := "--buildFlag='-UseModernBuildSystem=YES'" |
There was a problem hiding this comment.
For consistency, please use YES and NO or 0 and 1, but let's not mix the two. According to this SO answer, both formats work: https://stackoverflow.com/a/51205222
| @@ -119,19 +119,29 @@ inputs: | |||
| description: |- | |||
| Root directory of your Cordova project, where your Cordova config.xml exists. | |||
| is_required: true | |||
| - options: --buildFlag="-UseModernBuildSystem=0" | |||
| - options: | |||
| opts: | |||
| title: "Options to append to the cordova-cli build command" | |||
| description: |- | |||
| Use this input to specify custom options, to append to the end of the cordova-cli build command. | |||
|
|
|||
| The new Xcode build system is now supported in cordova-ios@5.0.0 (https://github.com/apache/cordova-ios/issues/407). | |||
There was a problem hiding this comment.
It's safe to delete this sentence too from the description.
There was a problem hiding this comment.
Yes it's safe because, it's only default "text" in input in this option, with new option you've a new system in default.
step.yml
Outdated
| Example: | ||
| - `--browserify` | ||
|
|
||
| `cordova build [OTHER_PARAMS] [options]` | ||
| - build_system: "modern" | ||
| opts: | ||
| title: "Build system" |
There was a problem hiding this comment.
| title: "Build system" | |
| title: "Xcode build system" |
step.yml
Outdated
| opts: | ||
| title: "Build system" | ||
| description: |- | ||
| The build system to use. |
There was a problem hiding this comment.
| The build system to use. | |
| The Xcode build system to use. |
CHANGELOG.md
Outdated
|
|
||
| ----------------- | ||
|
|
||
| ### 1.2.2 (2021 Oct 04) |
There was a problem hiding this comment.
We are changing the default behavior of the step here, so let's call this version 2.0.0. The legacy build system was the default, so it's a breaking change for those who used the default step inputs.
There was a problem hiding this comment.
Thank you for the contribution @manuelabarca! The changes look good, I just left a few minor comments about versioning and naming things. Looking forward to merge this PR!
manuelabarca
force-pushed
the
update-build-system
branch
from
00e8843
to
4ba9d05
Compare
The changes have been applied, thank you very much. |
|
PR it's ok ? Or wait other check ? |
|
Yes, the PR looks good, I just had to fix a broken test in CI (it's not related to the changes in this PR). |
…out arguments
Checklist
step.ymlandREADME.mdis updated with the changes (if needed)Version
2.0.0
Context
I have created a new option within the steps so that the user can decide if he wants to use the legacy system or the new system, I have taken advantage of the same thing that the "options" option occupies to be able to send the arguments but in a more friendly way
Resolves: #50
Changes
Changes have been made to the step.yml and main.go to be able to use this new option that allows the user to switch between xcode build systems.
Investigation details
Decisions