-
-
Notifications
You must be signed in to change notification settings - Fork 112
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
chore(linux): Add coverage action to ibus-keyman/build.sh
#9583
Conversation
User Test ResultsTest specification and instructions User tests are not required |
7798f73
to
a870b9a
Compare
linux/ibus-keyman/build.sh
Outdated
@@ -19,11 +19,16 @@ builder_describe \ | |||
"test" \ | |||
"install install artifacts" \ | |||
"uninstall uninstall artifacts" \ | |||
"report create coverage report" \ |
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.
Rather than making a new action for report
, could it be an option --report
inside the test
action?
(The coverage report depends on test being run, right?)
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.
Done. Seems a bit weird to call ninja
in response to an option, but it works.
This addresses code review comments.
linux/ibus-keyman/build.sh
Outdated
builder_describe_internal_dependency \ | ||
report:engine test:engine | ||
|
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.
builder_describe_internal_dependency \ | |
report:engine test:engine |
Now that --coverage
is an option checked inside the test action, this can go away, right?
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.
Oops. Thanks!
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
Addresses code review comment.
Changes in this pull request will be available for download in Keyman version 17.0.177-alpha |
@keymanapp-test-bot skip