-
Notifications
You must be signed in to change notification settings - Fork 42
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
Exit process with 1 when Flow check fails? #147
Comments
Any update here, @rpl? |
@migueloller Since @rpl doesn't seem to have a look at the issues, I have forked the project and implemented the exit code for flow: https://github.com/fintory/flow-coverage-report |
@iduuck, that looks great! Thanks for taking the initiative 😄 I'm not sure how responsive @rpl is to PRs but I'm sure it would be helpful if you submit one with the changes done in fintory@9c4a38c. You can likely just setup a branch, cherry-pick that commit and submit the PR. It'll get my +1 for sure! |
@migueloller @iduuck I personally prefer to have a flow-check npm script that fails when there are flowtype errors and leave the report generator to only fail because the coverage is lower than the desired threshold. But I'm not against supporting the behavior described in #147 (comment) and I would be more than happy on integrating it as an additional configurable behavior (enabled by an additional cli/config option, instead of being the default behavior, and possibly guarded by a new test case to prevent the new option and related behavior from regressing in the future, the modules around the cli argument parsing based on yargs are unfortunately the parts on which the flow coverage is lower and so, a bit ironically :-P, flow doesn't help a lot there). |
Ok, I did a PR (#158) for the default behaviour, but will add a config/cli option when I have some time. |
Currently it's possible to set a coverage threshold. It would be nice if there was another option that made it required for the Flow check to pass.
The text was updated successfully, but these errors were encountered: