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

Optimization to avoid re-fetching the node on each resolver call #684

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Optimization to avoid re-fetching the node on each resolver call #684

merged 1 commit into from
Apr 16, 2021

Conversation

odelalleau
Copy link
Collaborator

Note that the node is now guaranteed to always be available.

This is my suggestion from #679 (comment)

@odelalleau
Optimization to avoid re-fetching the node on each resolver call
4f6fcd8 
Note that the node is now guaranteed to always be available.
omry
omry reviewed Apr 16, 2021
View reviewed changes
Copy link
Owner

@omry omry 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.
Can you explain how you were able to make the node non-optional? was the case for None node only coming from the grammar test?

@odelalleau
Copy link
Collaborator Author

Can you explain how you were able to make the node non-optional? was the case for None node only coming from the grammar test?

Yes actually the only place where node was None in the previous code was in these grammar tests, due to not providing the key.

omry
omry approved these changes Apr 16, 2021
View reviewed changes
Copy link
Owner

@omry omry left a comment

Choose a reason for hiding this comment

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

shipit.

@odelalleau odelalleau merged commit 520841e into omry:master Apr 16, 2021
@odelalleau odelalleau deleted the pass_node branch April 16, 2021 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omry omry omry 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
@odelalleau @omry