Skip to content
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

Rework command line handling of build tool #2373

Merged
merged 1 commit into from Jun 6, 2018

Conversation

akosthekiss
Copy link
Member

In tools/build.py:

  • For the sake of readability, group CLI arguments as general build
    options, options to control the building of components, and
    component-specific options.
  • To prevent duplications, remove the defaults from those CLI
    arguments that correspond to CMake options and have defaults in
    any of the CMakeLists. Should any of the defaults change, they
    will have to be changed at a single place only. (Those options
    that are not set on the command line of tools/build.py are not
    passed as options to cmake either.)
  • Convert --unittests and --doctests to ON/OFF options like the
    rest of the component switches.
  • Touch on some of the help messages of the CLI arguments.

Other changes:

  • The change in --unittests and --doctests is a slightly CLI-
    breaking change of tools/build.py. Thus, follow up on this in
    tools/run-tests.py.
  • Move ENABLE_ALL_IN_ONE into jerry-core as it is not a general
    option but specific to that component.
  • Remove the forcing of ENABLE_ALL_IN_ONE for some compilers/
    platforms as it is still an option, not a hard requirement.

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu

@akosthekiss akosthekiss added enhancement An improvement tools Related to the tooling scripts labels Jun 2, 2018
Copy link
Contributor

@robertsipka robertsipka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akosthekiss
Copy link
Member Author

Note (to self/maintainers): this will conflict with #2370. Whichever PR gets landed first, the other will have to be rebased and conflicts resolved.

@yichoi
Copy link
Contributor

yichoi commented Jun 4, 2018

@akosthekiss rebase please

@@ -319,7 +319,7 @@ max-returns=6
max-branches=15

# Maximum number of statements in function / method body
max-statements=50
max-statements=75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

@akosthekiss akosthekiss force-pushed the rework-build-cli branch 2 times, most recently from 9505d9c to 845d3f2 Compare June 5, 2018 12:03
In `tools/build.py`:
- For the sake of readability, group CLI arguments as general build
  options, options to control the building of components, and
  component-specific options.
- To prevent duplications, remove the defaults from those CLI
  arguments that correspond to CMake options and have defaults in
  any of the CMakeLists. Should any of the defaults change, they
  will have to be changed at a single place only. (Those options
  that are not set on the command line of `tools/build.py` are not
  passed as options to `cmake` either.)
- Convert `--unittests` and `--doctests` to ON/OFF options like the
  rest of the component switches.
- Touch on some of the help messages of the CLI arguments.

Other changes:
- The change in `--unittests` and `--doctests` is a slightly CLI-
  breaking change of `tools/build.py`. Thus, follow up on this in
  `tools/run-tests.py`.
- Move `ENABLE_ALL_IN_ONE` into `jerry-core` as it is not a general
  option but specific to that component.
- Remove the forcing of `ENABLE_ALL_IN_ONE` for some compilers/
  platforms as it is still an option, not a hard requirement.

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
@akosthekiss
Copy link
Member Author

I've rebased and conflict-resolved the PR.

Copy link
Contributor

@yichoi yichoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yichoi yichoi merged commit b61d0ed into jerryscript-project:master Jun 6, 2018
@akosthekiss akosthekiss deleted the rework-build-cli branch June 6, 2018 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement tools Related to the tooling scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants