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

Melange v3 long-lived branch #821

Merged
merged 11 commits into from
Feb 7, 2024
Merged

Melange v3 long-lived branch #821

merged 11 commits into from
Feb 7, 2024

Conversation

anmonteiro
Copy link
Member

No description provided.

@anmonteiro anmonteiro marked this pull request as ready for review February 1, 2024 21:36
Copy link
Collaborator

@jchavarri jchavarri left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! I'm a bit confused about the changes in locations, and why I updated the merlin version at some point, I assume there was some incompatibility with Melange v3 or other package 🤔

"type": "(~greeting: string) => React.element",
"type": "(~greeting: string) => element",
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changes are strange, I assume is the new merlin version used? I can't see anything in reason-react itself that changed, or in melange.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand why these changed, but they're both correct and consistent across opam / nix calls, so I didn't worry too much.

},
"end": {
"line": 10,
"col": 85
},
"type": "{.. \"author\": Author.t} => React.element",
"type": "string",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also strange, but I guess now it's more accurate (I think this test is hovering over key prop, from what I see in line 21).

@anmonteiro anmonteiro merged commit f9052c1 into main Feb 7, 2024
3 checks passed
@anmonteiro anmonteiro deleted the anmonteiro/melange-v3 branch February 7, 2024 01:52
@anmonteiro anmonteiro restored the anmonteiro/melange-v3 branch February 7, 2024 02:15
@anmonteiro
Copy link
Member Author

I went ahead and restored the branch here. If your builds somehow failed because this melange-v3 branch was missing, please retry the build!

@jchavarri jchavarri deleted the anmonteiro/melange-v3 branch February 7, 2024 08:08
@jchavarri jchavarri restored the anmonteiro/melange-v3 branch February 7, 2024 08:08
davesnx added a commit to davesnx/opam-repository that referenced this pull request Feb 10, 2024
CHANGES:

* Wrap the `React` library, exposing just a single top-level module
  (@anmonteiro in [reasonml/reason-react#783](reasonml/reason-react#783))
* Re-organise toplevel modules (@davesnx in [reasonml/reason-react#794](reasonml/reason-react#794))
* Require and adapt to Melange v3 (@anmonteiro in [reasonml/reason-react#821](reasonml/reason-react#821))
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.

None yet

2 participants