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

feat(ts/hooks/useDetour): replace state variable with state machine #2616

Merged
merged 3 commits into from
May 23, 2024

Conversation

firestack
Copy link
Member

@firestack firestack commented May 22, 2024

Splitting #2615 up into two PR's, while trying to preserve context.

#2607 (comment)

Asana Ticket: https://app.asana.com/0/0/1207381767522633/f

Copy link

Coverage of commit e8b1719

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link
Collaborator

@hannahpurcell hannahpurcell left a comment

Choose a reason for hiding this comment

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

This makes sense to me from an iterative perspective. (So while the Idle/Done split doesn't seem to be needed here, I know it's developed further in the later PR, so I'm approving this one and looking more closely at the subsequent one. Function otherwise looks good.) Noted one proposed wording change, but non-blocking.

@firestack firestack force-pushed the kf/asn/state-machine/state-var branch from e8b1719 to c5999e0 Compare May 23, 2024 12:01
Copy link

Coverage of commit c5999e0

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@firestack firestack force-pushed the kf/asn/state-machine/state-var branch from c5999e0 to ed07886 Compare May 23, 2024 12:20
Copy link

Coverage of commit ed07886

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@firestack firestack requested a review from joshlarson May 23, 2024 12:28
Copy link
Contributor

@joshlarson joshlarson 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 to me!

I made a comment that I'm interested in your thoughts on, but 👍 either way.

@firestack firestack force-pushed the kf/asn/state-machine/state-var branch from ed07886 to 5f13e11 Compare May 23, 2024 16:11
@firestack firestack force-pushed the kf/asn/state-machine/state-var branch from 5f13e11 to d933c71 Compare May 23, 2024 16:14
@firestack firestack force-pushed the kf/asn/state-machine/state-var branch from d933c71 to cfdc141 Compare May 23, 2024 16:19
@firestack firestack enabled auto-merge (squash) May 23, 2024 16:19
@firestack firestack merged commit 99eddb7 into main May 23, 2024
9 checks passed
@firestack firestack deleted the kf/asn/state-machine/state-var branch May 23, 2024 16:24
Copy link

Coverage of commit cfdc141

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

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.

3 participants