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

Fix the measure_coverage.sh script #1408

Merged
merged 1 commit into from Jan 12, 2018

Conversation

rtakacs
Copy link
Contributor

@rtakacs rtakacs commented Jan 11, 2018

Removed the old --no-check-test option from the build flags.

@robertsipka
Copy link
Contributor

robertsipka commented Jan 11, 2018

Could you please also remove it from other places (e.g. Build-Script.md) ? Otherwise, LGTM (informally)

@rtakacs
Copy link
Contributor Author

rtakacs commented Jan 11, 2018

@robertsipka Nice catch! Of course :)

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

IoT.js-DCO-1.0-Signed-off-by: Roland Takacs rtakacs.uszeged@partner.samsung.com
@rtakacs
Copy link
Contributor Author

rtakacs commented Jan 11, 2018

@robertsipka I've updated the files to have the new run-test option instead of the old no-check-test one.

@@ -235,6 +227,14 @@ To build for nuttx os, nuttx home directory must be given.
./tools/build.py --target-os=nuttx --target-arch=arm --target-board=stm32f4dis --nuttx-home="..."
```

--
#### `--run-test`
With given this option, unit test checking will be performed.
Copy link
Contributor

Choose a reason for hiding this comment

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

@rtakacs Although it is not a part of this PR, what about "run unit tests after build"? I would remove all occurrences of With given this option in explanation for each option. Without this phrases, it is still clear the each sentence explains the given option.

Copy link
Contributor

@glistening glistening left a comment

Choose a reason for hiding this comment

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

LGTM (The comment above is optional, because it is out of scope of this PR)

@yichoi yichoi merged commit d06718a into jerryscript-project:master Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants