Skip to content

Conversation

zach-klippenstein
Copy link
Collaborator

See the documentation on the WorkflowInterceptor interface for an explanation of what this
type is and how it can be used. It is intended to replace WorkflowDiagnosticListener, as
well as the custom double-rendering behavior used to verify render idempotency in testing
infrastructure.

Closes #33.

Checklist

  • Unit Tests
  • UI Tests
  • I have made corresponding changes to the documentation

@zach-klippenstein zach-klippenstein added this to the v1.0.0 milestone Jun 25, 2020
@zach-klippenstein zach-klippenstein requested a review from a team as a code owner June 25, 2020 01:50
@zach-klippenstein zach-klippenstein force-pushed the zachklipp/workflow-interceptor branch from 36f97c9 to f840429 Compare June 25, 2020 02:03
* is intercepting.
*/
@ExperimentalWorkflowApi
interface WorkflowInstance {
Copy link
Collaborator Author

@zach-klippenstein zach-klippenstein Jun 25, 2020

Choose a reason for hiding this comment

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

Is this name confusing? It's hard to talk about "instances of Workflow classes" vs this. We typically call these "sessions" offline, should we just use that term here? Also cc @square/mobile-foundation-ios for their thoughts on this. (Scroll up to the documentation on WorkflowInterceptor to read a detailed description of what this is.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Glad you asked, I was biting my tongue. I prefer Session.

* also knows the [key][WorkflowInstance.renderKey] used to render the workflow and the
* [WorkflowInstance] of the [parent][WorkflowInstance.parent] workflow that is rendering it.
*
* Each instance is also assigned a numerical ID that uniquely identifies the instance over the
Copy link
Collaborator

Choose a reason for hiding this comment

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

pedant: what if it wraps?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, it's a Long. Wrapping those is…challenging. Although we did it once at Google.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These track in-memory objects. If you wrap that you've got more than 18446744073 GB of RAM in your phone so good for you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"18 ZB ought to be enough for anybody." a famous person once said, I think.

* is intercepting.
*/
@ExperimentalWorkflowApi
interface WorkflowInstance {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Glad you asked, I was biting my tongue. I prefer Session.

@zach-klippenstein zach-klippenstein force-pushed the zachklipp/workflow-interceptor branch from f840429 to b0344ed Compare June 25, 2020 21:15
@zach-klippenstein
Copy link
Collaborator Author

Renamed Instance to Session and added some documentation.

See the documentation on the `WorkflowInterceptor` interface for an explanation of what this
type is and how it can be used. It is intended to replace `WorkflowDiagnosticListener`, as
well as the custom double-rendering behavior used to verify render idempotency in testing
infrastructure.

Closes #33.
@zach-klippenstein zach-klippenstein force-pushed the zachklipp/workflow-interceptor branch from b0344ed to cad6331 Compare June 25, 2020 22:05
@zach-klippenstein zach-klippenstein merged commit 998abd0 into main Jun 25, 2020
@zach-klippenstein zach-klippenstein deleted the zachklipp/workflow-interceptor branch June 25, 2020 22:31
zach-klippenstein added a commit that referenced this pull request Feb 4, 2021
Verified that #42 has been fixed in dev16. Closes #42.
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.

Proposal: Replace diagnostic API with something more similar to okhttp's interceptors
2 participants