-
Notifications
You must be signed in to change notification settings - Fork 136
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.
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
add ActionResult::SustainConnection
to MG
#1024
add ActionResult::SustainConnection
to MG
#1024
Conversation
ActionResult::SustainConnection
to MG
Bencher
🚨 1 ALERT: Threshold Boundary Limit exceeded!
Click to view all benchmark results
Bencher - Continuous Benchmarking View Public Perf Page Docs | Repo | Chat | Help |
Bencher
🚨 1 ALERT: Threshold Boundary Limit exceeded!
Click to view all benchmark results
Bencher - Continuous Benchmarking View Public Perf Page Docs | Repo | Chat | Help |
Bencher
Click to view all benchmark results
Bencher - Continuous Benchmarking View Public Perf Page Docs | Repo | Chat | Help |
Bencher
🚨 10 ALERTS: Threshold Boundary Limits exceeded!
Click to view all benchmark results
Bencher - Continuous Benchmarking View Public Perf Page Docs | Repo | Chat | Help |
70d395c
to
492c1c8
Compare
Can you add something about this new check in the readme? How and when to use it? |
I'm looking around but I cannot find any place where the other Action Results are described. Should we create a new section on |
yep I would add it in the action section |
great, I added a new issue #1027 for this, since I don't think it makes sense to document all possible Action Results under this PR. I also wrote a comment about rationale and usage of For now, @Fi3 would you mind carrying on with the review on this PR without those docs? It would be good to get it merged in order to unblock #1025 |
My only concern was adding something in readme about why we need it and how to use. Is ok with me to add it later. Everything else LGTM. |
69c58a6
to
d8cd5e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah nice, this looks great. Added few small comments
d8cd5e6
to
43512cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm. Definitely more readable.
43512cc
to
43edba3
Compare
43edba3
to
6fcdc1c
Compare
close #1023 and unblock #1025
this allows for MG to verify that a connection was not closed
it also refactors some parts of
Executor::execute
by moving some logic to the arms ofmatch result
, which improves code readability