Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

API update to support the newly added reexport field. #1718

Open
wants to merge 10 commits into
base: wip/ao/api-update
Choose a base branch
from

Conversation

mwu-tow
Copy link
Contributor

@mwu-tow mwu-tow commented Jul 19, 2021

Pull Request Description

IDE support for enso-org/enso#1793

Important Notes

This PR is based on Adam's API updated, as they are necessary for newer engine versions.
Will refrain from testing scenarios until "actual" merge is possible.
New field is not actually "used" (as in leading to visible user change), this PR only introduces API support.

Checklist

Please include the following checklist in your PR:

~- [ ] The CHANGELOG.md was updated with the changes introduced in this PR. ~

  • The documentation has been updated if necessary.
  • All code conforms to the Rust style guide.
  • All code has automatic tests where possible.
    - [ ] All code has been profiled where possible.
  • All code has been manually tested in the IDE.
    - [ ] All code has been manually tested in the "debug/interface" scene.
  • All code has been manually tested by the PR owner against our test scenarios.
  • All code has been manually tested by at least one reviewer against our test scenarios.

@mwu-tow mwu-tow added Type: Enhancement An enhancement to the current state of Enso IDE Category: Controllers The Application layer not bound to visual part labels Jul 19, 2021
@mwu-tow mwu-tow self-assigned this Jul 19, 2021
@mwu-tow mwu-tow requested a review from farmaazon as a code owner July 19, 2021 18:10
@mwu-tow mwu-tow requested a review from wdanilo as a code owner July 20, 2021 07:48
Copy link
Collaborator

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

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

The only thing I don't like is the name. The export suggest that this an exporting line, and adding re- is confusing even more. Does if the module containing the object "re-export" this? Or maybe: does it mean, that the field will be filled only when the object is re-exported, and will be empty if only module containing it export it?

@4e6
Copy link
Contributor

4e6 commented Jul 20, 2021

The logic behind the re-export naming is that every module implicitly exports all its definitions. Another module can re-export symbols that were defined in a different module.
In other words, module B takes the symbol exported from module A and re-exports this symbol with a new module name.

So, this re prefix only tells that this module is not the one where the symbol was defined.

@farmaazon
Copy link
Collaborator

So, this re prefix only tells that this module is not the one where the symbol was defined.

Ok, so in case when no module re-export given symbol, the reexport field will be empty and we ought to make import basing on the module field, right?

@MichaelMauderer
Copy link
Contributor

@mwu-tow What is the status of this PR? Should it be updated/finished, or is it abandoned and should be closed?

@mwu-tow
Copy link
Contributor Author

mwu-tow commented Sep 20, 2021

@MichaelMauderer
TBH I'm confused here. PR was ready to merge, apart from waiting review by Wojciech and next release bump.
However, it seems that #1726 actually duplicated parts of it, so now it is just mess.

Likely the missing pieces should be manually merged back.

@MichaelMauderer
Copy link
Contributor

@MichaelMauderer
TBH I'm confused here. PR was ready to merge, apart from waiting review by Wojciech and next release bump.
However, it seems that #1726 actually duplicated parts of it, so now it is just mess.

Likely the missing pieces should be manually merged back.

This PR must have fallen through the cracks during the release process back then.

Can you bring this PR up to date or create a new PR with the things that still need to be merged? (After checking with planning about priorities)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Category: Controllers The Application layer not bound to visual part Type: Enhancement An enhancement to the current state of Enso IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants