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

Allow for importing methods #3633

Merged
merged 8 commits into from
Aug 5, 2022
Merged

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented Aug 4, 2022

Pull Request Description

Importing individual methods didn't work as advertised because parser
would allow them but later drop that information. This slipped by because we never had mixed atoms and methods in stdlib.

Important Notes

Added some basic tests but we need to ensure that the new parser allows for this.
@jdunkerley will be adding some changes to stdlib that will be testing this functionality as well.

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 ide dist and ./run ide watch.

Importing individual methods didn't work as advertised because parser
would allow them but later drop that information.
@hubertp hubertp force-pushed the wip/hubert/import-module-methods branch from e82ff75 to 989fbde Compare August 4, 2022 14:18
@hubertp hubertp marked this pull request as ready for review August 4, 2022 14:19
@hubertp hubertp requested a review from 4e6 as a code owner August 4, 2022 14:19
@hubertp hubertp force-pushed the wip/hubert/import-module-methods branch from 989fbde to d6d12d2 Compare August 4, 2022 14:20
@hubertp hubertp requested a review from jdunkerley August 4, 2022 14:20
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.

LGTM but needs @4e6 review :)

@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Aug 4, 2022
@hubertp
Copy link
Contributor Author

hubertp commented Aug 5, 2022

Figured out how to add some basic tests. This is gtg

@hubertp hubertp requested a review from radeusgd as a code owner August 5, 2022 15:00
@mergify mergify bot merged commit 42dbd8b into develop Aug 5, 2022
@mergify mergify bot deleted the wip/hubert/import-module-methods branch August 5, 2022 16:25
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.

4 participants