Skip to content

Clean up SendLinkStep#5772

Merged
zachmargolis merged 2 commits intomainfrom
margolis-link-sent-feedback
Jan 3, 2022
Merged

Clean up SendLinkStep#5772
zachmargolis merged 2 commits intomainfrom
margolis-link-sent-feedback

Conversation

@zachmargolis
Copy link
Contributor

- PR feedback from #5760

Co-authored-by: Andrew Duthie <aduth@users.noreply.github.com>
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Any thoughts about the other comment at #5760 (comment) ?

Co-authored-by: Andrew Duthie <aduth@users.noreply.github.com>
@zachmargolis
Copy link
Contributor Author

Any thoughts about the other comment at #5760 (comment) ?

Yup, check out f4eb475

The regression specs pass! Huge simplification, thanks

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Nice, looks good 👍


def render_document_capture_cancelled
failure(I18n.t('errors.doc_auth.document_capture_cancelled'))
mark_steps_incomplete
Copy link
Contributor

Choose a reason for hiding this comment

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

Arguably we could remove link_sent from HYBRID_FLOW_STEPS since it's not having any effect here anyways, but I don't think it hurts anything to keep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! I have no desire to touch this code any more than is absolutely needed, gonna leave as-is for now

@zachmargolis zachmargolis merged commit d295f45 into main Jan 3, 2022
@zachmargolis zachmargolis deleted the margolis-link-sent-feedback branch January 3, 2022 20:37
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