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

Move workflow utility components to separate package #4015

Merged
merged 6 commits into from
Feb 26, 2021

Conversation

yycptt
Copy link
Contributor

@yycptt yycptt commented Feb 25, 2021

What changed?

  • Move workflow utility components to separate package
  • Added unit test for util functions to update workflow execution and get domain entry
  • Renamed some util functions

Why?
#3181

How did you test it?
Existing tests.

Potential risks
Risk is minimal as it only moves code around.

@yycptt yycptt requested review from yux0 and a team February 25, 2021 03:09
@yycptt yycptt marked this pull request as ready for review February 25, 2021 03:26
@@ -272,12 +288,11 @@ func GetActiveDomainEntry(
domainUUID string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this move to shard context?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1
This would makes more sense as it does not have anything workflow related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was thinking probably this should be moved to domain package instead of shard as it has not much to do with shard, which is only for getting the domain cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Synced with @yux0 offline. So GetActivityDomainEntry now is a method of the DomainCache interface and ValidateDomainUUID is moved to common package.

@yycptt yycptt force-pushed the workflow-util-package branch from 5c268c6 to b5e4baa Compare February 26, 2021 01:32
@yycptt yycptt force-pushed the workflow-util-package branch from 3ee8e16 to 1e0e3f0 Compare February 26, 2021 01:33
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 64.611% when pulling 1e0e3f0 on yycptt:workflow-util-package into 9422371 on uber:master.

@yycptt yycptt merged commit 80b7bfc into cadence-workflow:master Feb 26, 2021
@yycptt yycptt deleted the workflow-util-package branch February 26, 2021 02:04
yux0 pushed a commit to yux0/cadence that referenced this pull request May 4, 2021
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.

4 participants