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.

Sign up for GitHub

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

Jump to bottom

Drop-down widgets for extension functions via UnresolvedSymbol #7115

Merged
merged 32 commits into from
Jun 27, 2023
Merged

Drop-down widgets for extension functions via UnresolvedSymbol #7115

merged 32 commits into from
Jun 27, 2023

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Jun 23, 2023 •
edited
Loading

Pull Request Description

Fixes #6955 by:

  • using visualisationModule to specify the module where the visualization is to be used
  • referring to method in Meta.get_annotation with .method_name - e.g. unresolved symbol notation
  • evaluating arguments to Meta.get_annotation in the context of the user module (which can access the extension functions)

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Jun 23, 2023
@JaroslavTulach JaroslavTulach self-assigned this Jun 23, 2023
@JaroslavTulach
Copy link
Member Author

primary_key is really drop down widget seems to be working:

obrazek

This was referenced Jun 23, 2023
Closed
Merged
@JaroslavTulach
Copy link
Member Author

Test failure improved in 673c49e

@JaroslavTulach
Copy link
Member Author

There are two failing tests:

@enso-bot enso-bot bot mentioned this pull request Jun 24, 2023
Closed
@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/UnresolvedSymbol_6955 branch from 50fdf9a to 45f5945 Compare June 27, 2023 07:36
@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/UnresolvedSymbol_6955 branch from 45f5945 to 6365b25 Compare June 27, 2023 07:39
hubertp
hubertp reviewed Jun 27, 2023
View reviewed changes
Copy link
Contributor

@hubertp hubertp left a comment

Choose a reason for hiding this comment

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

Some missing documentation (in the code). Other than that, engine part looks fine.

app/gui/src/controller/graph/executed.rs Show resolved Hide resolved
app/gui/src/controller/graph/executed.rs Outdated Show resolved Hide resolved
engine/polyglot-api/src/main/scala/org/enso/polyglot/runtime/Runtime.scala Show resolved Hide resolved
...ument-common/src/main/scala/org/enso/interpreter/instrument/job/UpsertVisualizationJob.scala Show resolved Hide resolved
@JaroslavTulach JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Jun 27, 2023
@JaroslavTulach
Copy link
Member Author

I believe I addressed all requests for comments and the PR is ready to be merged. Please approve.

@JaroslavTulach JaroslavTulach removed the CI: Ready to merge This PR is eligible for automatic merge label Jun 27, 2023
4e6
4e6 approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@4e6 4e6 left a comment

Choose a reason for hiding this comment

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

I left a couple of comments on what I believe needs to be fixed. Other than that LGTM

@JaroslavTulach JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Jun 27, 2023
@mergify mergify bot merged commit 477dd82 into develop Jun 27, 2023
25 checks passed
@mergify mergify bot deleted the wip/jtulach/UnresolvedSymbol_6955 branch June 27, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radeusgd radeusgd radeusgd approved these changes

@farmaazon farmaazon farmaazon approved these changes

@4e6 4e6 4e6 approved these changes

@Frizi Frizi Frizi approved these changes

@jdunkerley jdunkerley jdunkerley approved these changes

@GregoryTravis GregoryTravis Awaiting requested review from GregoryTravis GregoryTravis is a code owner

@Akirathan Akirathan Awaiting requested review from Akirathan Akirathan is a code owner

@MichaelMauderer MichaelMauderer Awaiting requested review from MichaelMauderer

@wdanilo wdanilo Awaiting requested review from wdanilo

@mwu-tow mwu-tow Awaiting requested review from mwu-tow

@kazcw kazcw Awaiting requested review from kazcw

@hubertp hubertp Awaiting requested review from hubertp hubertp is a code owner

Assignees

@JaroslavTulach JaroslavTulach

Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@widgets for extension functions of Table may not be found
7 participants
@JaroslavTulach @hubertp @4e6 @Frizi @radeusgd @farmaazon @jdunkerley