[go: nahoru, domu]
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

Always Send Panic Updates #1508

Merged
merged 7 commits into from
Feb 25, 2021
Merged

Always Send Panic Updates #1508

merged 7 commits into from
Feb 25, 2021

Conversation

4e6
Copy link
Contributor
@4e6 4e6 commented Feb 24, 2021 •
edited
Loading

Pull Request Description

close #1506

Changelog:

  • feat: cache error values of all expressions
  • feat: send runtime update if the error value is changed
  • feat: always send panic updates

Important Notes

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 documentation and configuration conforms to the markdown and YAML style guides.
  • All code has been tested where possible.

@4e6 4e6 added Type: Enhancement p-highest Should be completed ASAP labels Feb 24, 2021
@4e6 4e6 self-assigned this Feb 24, 2021
kustosz
kustosz suggested changes Feb 24, 2021
View reviewed changes
Copy link
Contributor
@kustosz kustosz left a comment

Choose a reason for hiding this comment

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

I don't understand why it is necessary to cache dataflow errors – these should be cached under normal rules. AFAIR the dataflow error update does not contain any error-specific data (that is instead handled through the normal vis framework), so there's no point resending that if the type does not change. We should have close to zero special logic for dataflow errors and everything should work fine.

@4e6 4e6 changed the title Cache Error Values Always Send Panic Updates Feb 25, 2021
@4e6 4e6 requested a review from kustosz February 25, 2021 07:00
@4e6 4e6 merged commit b79d5f9 into main Feb 25, 2021
@4e6 4e6 deleted the wip/db/panic-updates branch February 25, 2021 10:43
iamrecursion pushed a commit that referenced this pull request Feb 26, 2021
@4e6 @iamrecursion
Always Send Panic Updates (#1508)
20ba42f 
Always send panic updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kustosz kustosz kustosz approved these changes

@iamrecursion iamrecursion Awaiting requested review from iamrecursion

@radeusgd radeusgd Awaiting requested review from radeusgd

Assignees

@4e6 4e6

Labels
p-highest Should be completed ASAP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Language Server does not send updates if panicing expression is changed to another panic.
2 participants
@4e6 @kustosz