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

Cleanup Ref - get/put #3457

Merged
merged 8 commits into from
May 17, 2022
Merged

Cleanup Ref - get/put #3457

merged 8 commits into from
May 17, 2022

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented May 16, 2022

Pull Request Description

The change promotes static methods of Ref, get and put, to be
methods of Ref type.
The change also removes Ref module from the default namespace.
Had to mostly c&p functional dispatch for now, in order for the methods
to be found. Will auto-generate that code as part of builtins system.

Related to https://www.pivotaltracker.com/story/show/182138899

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Scala, Java, and Rust style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH ./run dist and ./run watch.

Copy link
Member

@jdunkerley jdunkerley 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 - just one thing to check then happy to approve

@hubertp hubertp marked this pull request as ready for review May 16, 2022 14:03
The change promotes static methods of `Ref`, `get` and `put`, to be
methods of `Ref` type.
The change also removes `Ref` module from the default namespace.
Had to mostly c&p functional dispatch for now, in order for the methods
to be found. Will auto-generate that code as part of builtins system.

Related to https://www.pivotaltracker.com/story/show/182138899
Solution proposed by @4e6. Can't write `Standard.Base.Runtime.Ref.new`
or import the module in that lambda so this is the only sensible
solution.
@hubertp hubertp force-pushed the wip/hubert/update-ref-182138899 branch from 8753639 to 785f19e Compare May 16, 2022 14:03
@hubertp hubertp requested a review from jdunkerley May 16, 2022 14:08
@hubertp
Copy link
Contributor Author

hubertp commented May 16, 2022

This probably only needs 👍 from @radeusgd and @4e6 more. The other reviews were requested automatically because I touched a file shared with IDE.

@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label May 17, 2022
@mergify mergify bot merged commit 6b6b143 into develop May 17, 2022
@mergify mergify bot deleted the wip/hubert/update-ref-182138899 branch May 17, 2022 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants