search

rpl / flow-coverage-report

Generate flow coverage reports in JSON, HTML and in the console.
MIT License
505 stars 42 forks source link

Exit process with 1 when Flow check fails? #147

Open migueloller opened 6 years ago

migueloller commented 6 years ago

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.

iduuck commented 6 years ago

Any update here, @rpl?

iduuck commented 6 years ago

@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

migueloller commented 6 years ago

@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 https://github.com/fintory/flow-coverage-report/commit/9c4a38c5e1f16afbacff44851381011fe2d5489b. You can likely just setup a branch, cherry-pick that commit and submit the PR. It'll get my +1 for sure!

rpl commented 6 years ago

@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 https://github.com/rpl/flow-coverage-report/issues/147#issue-319332235 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).

iduuck commented 6 years ago

Ok, I did a PR (https://github.com/rpl/flow-coverage-report/pull/158) for the default behaviour, but will add a config/cli option when I have some time.