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.

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

#3673: add retry and random bricks #3674

Merged
merged 3 commits into from
Jun 13, 2022
Merged

Conversation

twschiller
Copy link
Contributor

@twschiller twschiller commented Jun 13, 2022

What does this PR do?

Discussion

  • I chose to implement "Retry" instead of "wait until" b/c it's a bit more natural way of framing the more control flow (e.g., retrying network requests). A user can implement "wait until" by raising a business/cancel error if their condition is not met

Future Work

Checklist

  • Add tests
  • Designate a primary reviewer: @BLoe

@twschiller twschiller requested a review from BLoe June 13, 2022 01:12
@twschiller twschiller self-assigned this Jun 13, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2022

Codecov Report

Merging #3674 (894676d) into main (105a613) will increase coverage by 0.06%.
The diff coverage is 69.38%.

@@            Coverage Diff             @@
##             main    #3674      +/-   ##
==========================================
+ Coverage   45.62%   45.68%   +0.06%     
==========================================
  Files         816      818       +2     
  Lines       24412    24460      +48     
  Branches     5143     5150       +7     
==========================================
+ Hits        11138    11175      +37     
- Misses      12357    12368      +11     
  Partials      917      917              
Impacted Files Coverage Δ
src/blocks/transformers/registerTransformers.ts 0.00% <0.00%> (ø)
src/development/headers.ts 0.00% <0.00%> (ø)
src/pageEditor/utils.ts 76.59% <33.33%> (-2.95%) ⬇️
src/blocks/transformers/randomNumber.ts 73.33% <73.33%> (ø)
src/blocks/transformers/controlFlow/Retry.ts 78.57% <78.57%> (ø)
src/utils.ts 72.03% <0.00%> (+0.84%) ⬆️
src/pageEditor/hooks/useExtensionTrace.ts 82.75% <0.00%> (+3.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 105a613...894676d. Read the comment docs.

@twschiller twschiller added this to the 1.7.0 milestone Jun 13, 2022
Copy link
Collaborator

@BLoe BLoe left a comment

Choose a reason for hiding this comment

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

Just a couple comments on the descriptions for random number, but LGTM otherwise 👍

src/blocks/transformers/controlFlow/Retry.ts Show resolved Hide resolved
Comment on lines 49 to 52
floating: {
type: "boolean",
description: "Specify returning a floating-point number.",
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could maybe be a little more clear to specify integer or float in the description.

Comment on lines 39 to 48
lower: {
type: "number",
description: "The lower bound",
default: 0,
},
upper: {
type: "number",
description: "The upper bound",
default: 1,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to specify inclusive/exclusive in the descriptions here.

@twschiller twschiller enabled auto-merge (squash) June 13, 2022 01:45
@twschiller twschiller merged commit 76dac91 into main Jun 13, 2022
@twschiller twschiller deleted the feature/3673-retry-brick branch June 13, 2022 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add retry control flow brick
3 participants