Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

feat(Scope): improve the support of local bindings #105

Merged
merged 3 commits into from
May 17, 2023
Merged

feat(Scope): improve the support of local bindings #105

merged 3 commits into from
May 17, 2023

Conversation

favonia
Copy link
Collaborator

@favonia favonia commented May 17, 2023 •
edited
Loading

This will be a big update...

  1. Deprecate Scope.modify in favor of Scope.modify_standalone because I feel "modify" is too ambiguous
  2. Add import_singleton to improve the support of local bindings.
  3. Fix and clean up more documentation

@favonia favonia requested a review from jonsterling May 17, 2023 16:25
@favonia favonia force-pushed the local-bindings branch 2 times, most recently from f938ed0 to 52b75ea Compare May 17, 2023 16:34
jonsterling
jonsterling reviewed May 17, 2023
View reviewed changes
Copy link
Contributor

@jonsterling jonsterling left a comment

Choose a reason for hiding this comment

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

This looks like a nice API, but I still want to understand why the version without this works. I wonder if we can write some tests to demonstrate the bad behavior?

@favonia
feat(Scope): improve the support of local bindings
ff43dd8 
1. Add the optimized `import_singleton` for local bindings
2. Fix the documentation of `section` and other functions.
@favonia favonia changed the title feat(Scope): support local bindings feat(Scope): improve the support of local bindings May 17, 2023
@favonia favonia requested a review from jonsterling May 17, 2023 17:26
jonsterling
jonsterling approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@jonsterling jonsterling left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for this!

@favonia favonia merged commit c12f9c5 into main May 17, 2023
@favonia favonia deleted the local-bindings branch May 17, 2023 18:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jonsterling jonsterling jonsterling approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@favonia @jonsterling