Skip to content

Add trigger token for executor usage#2391

Merged
daved merged 4 commits intoversion/0-37-0-RC1from
green/add_exec_trigger.DX-1588
Mar 1, 2023
Merged

Add trigger token for executor usage#2391
daved merged 4 commits intoversion/0-37-0-RC1from
green/add_exec_trigger.DX-1588

Conversation

@daved
Copy link
Copy Markdown
Contributor

@daved daved commented Feb 23, 2023

StoryDX-1588 Separate trigger for `state exec` and executors

@daved
Copy link
Copy Markdown
Contributor Author

daved commented Feb 23, 2023

@Naatan In the AC, the state exec command is to get a new token as "exec-cmd". However this would then not match with other commands like activate which do not use that suffix. I added a new token for executors simply named "executor". I'm happy to redo the changes to match AC more exactly if the odd "-cmd" suffix is desirable, or, alternatively, the new "executor" token can be renamed to be more distinct from the existing "exec" token.

@daved daved requested a review from Naatan February 23, 2023 18:19
Copy link
Copy Markdown
Contributor

@Naatan Naatan left a comment

Choose a reason for hiding this comment

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

Please stick to the original AC's. I agree the naming inconsistency is awkward, but ultimately inconsequential and it's better than having DW have to handle runtime tracking using a different trigger for some version or date cutoff.

I'm always happy to chat on Slack if AC's are posing problems. Rather we have a quick chat and adjust the AC's before investing time on implementation.

Comment thread pkg/platform/runtime/target/target.go Outdated
@daved daved requested a review from Naatan February 24, 2023 18:42
@daved daved requested a review from Naatan February 28, 2023 23:51
@daved daved merged commit 92de4e4 into version/0-37-0-RC1 Mar 1, 2023
@daved daved deleted the green/add_exec_trigger.DX-1588 branch March 1, 2023 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants