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 --enable-outside-detected-project #1156

Merged
merged 4 commits into from
Nov 19, 2019
Merged

Conversation

Julow
Copy link
Collaborator

@Julow Julow commented Nov 18, 2019

Fix #1154
Bug introduced in #1127 (on master: 45f324)

The parsing of option has been moved after that option is checked, at toplevel.
Same for the --root option, but I didn't manage to make it work yet while writing tests.

…rking

Since 45f324, parsing of option is done later in that file, after those options are checked at toplevel.
@Julow Julow requested a review from gpetiot November 18, 2019 16:47
@facebook-github-bot
Copy link

Hi Julow! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@Julow Julow requested a review from emillon November 18, 2019 16:47
@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Copy link
Collaborator

@emillon emillon left a comment

Choose a reason for hiding this comment

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

Thanks. I think that this testing approach is the way to go. 👍

CHANGES.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@gpetiot gpetiot left a comment

Choose a reason for hiding this comment

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

Great, looks good.

@Julow Julow merged commit a906b5d into ocaml-ppx:master Nov 19, 2019
bogdan2412 pushed a commit to bogdan2412/ocamlformat that referenced this pull request Mar 28, 2020
* Add tests for --enable-outside-detected-project
* Fix --enable-outside-detected-project and --root options no longer working
Since 45f324, parsing of option is done later in that file, after those options are checked at toplevel.
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.

Bug: --enable-outside-detected-project flag does not work
4 participants