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

Run bisect conditionally #20

Merged
merged 1 commit into from
Apr 13, 2020
Merged

Run bisect conditionally #20

merged 1 commit into from
Apr 13, 2020

Conversation

wyze
Copy link
Owner

@wyze wyze commented Apr 13, 2020

Close #19

@wyze wyze merged commit 74e4e3a into master Apr 13, 2020
@wyze wyze deleted the conditional-bisect branch April 13, 2020 14:07
@jihchi
Copy link
Contributor

jihchi commented Apr 26, 2020

Hi @wyze ,

Sorry to hijack this merged PR. I am not sure if you are aware of this, just wanted to for your information:

bisect_ppx has to be a regular dep; see aantron/bisect_ppx#309 (comment). This is because the ppx-flags in bsconfig.json cause Bisect to always be called. It's just when BISECT_ENABLE=yes is missing, Bisect leaves the code clean and uninstrumented. But the code still gets passed through Bisect.

Once BuckleScript releases dev ppx-flags (rescript-lang/rescript-compiler#3761) in the next release, it will be correct to move bisect_ppx to devDependencies, which is where it properly belongs :)

@wyze
Copy link
Owner Author

wyze commented Apr 27, 2020

@jihchi Yup I saw it. I am waiting for the dev-ppx-flags as well. I have a small package that runs on CI only that updates the bsconfig.json and adds the ppx-flags and bisect_ppx to bs-dependencies. So far working good!

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.

Error: package bisect_ppx not found or built
2 participants