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

Parse and print braces around inline records #2363

Merged

Conversation

anmonteiro
Copy link
Member

This was brought up in #2336

@anmonteiro
Copy link
Member Author

We should also fix this in the outcome printer when #2336 is in. I don't wanna have to deal with merge conflicts :)

@anmonteiro anmonteiro force-pushed the anmonteiro/braces-inline-records branch from 1df5cb9 to 794e57c Compare March 31, 2019 10:39
@jordwalke
Copy link
Member

This is great! It's a "minor version bump" change, which means it's a new feature that people might depend on in their code.

For example, for this we would bump the version from 3.4.1 to 3.5.0. No one's existing code will break (that would be a 4.x.x bump) but some people can write code that depends on the new feature so it's an y bump in x.y.z (that's called a "minor version" bump).

If John uses this feature in his code, he will need to specify in their package versions:

"@esy-ocaml/reason": ">= 3.5.0 < 4.0.0"

If Mary has a package that depends on "end of line comments" feature, she would have specified:

"@esy-ocaml/reason": ">= 3.4.0 < 4.0.0"

Because `3.4.0 added end of line comments.

And if those two people mixed their packages into an app, the package solver would know to use reason version 3.5.0 with your change. I just thought I'd write out the the steps of how I think through potential scenarios for breakage so other people understand what's behind versioning in Reason.

@jordwalke jordwalke merged commit 8e53e86 into reasonml:master Apr 1, 2019
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Apr 1, 2019
This is a followup to the syntax change added in reasonml#2363, but for the
outcome printer
jordwalke pushed a commit that referenced this pull request Apr 1, 2019
This is a followup to the syntax change added in #2363, but for the
outcome printer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants