Skip to content

Conversation

@mpneuried
Copy link
Member

No description provided.

@mpneuried mpneuried requested review from bdebinska and iStefo June 16, 2025 14:27
@mpneuried mpneuried self-assigned this Jun 16, 2025
@mpneuried mpneuried added the enhancement New feature or request label Jun 16, 2025
@mpneuried mpneuried marked this pull request as ready for review June 16, 2025 14:46
Copy link
Member

@bdebinska bdebinska 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, a couple of things related to this PR:

The comment in this line should be changed: https://github.com/box-id/workflow_engine/blob/main/lib/actions/action.ex#L4

It would be nice to provide the actual github ssh links here, so that we can just copy them over:
https://github.com/box-id/workflow_engine_actions/blob/main/README.md?plain=1#L25

Do the actions need to be nested in the lib/actions? They could just live in the lib IMO https://github.com/box-id/workflow_engine_actions/tree/main/lib/actions

Do we use this anywhere?
https://github.com/box-id/workflow_engine_actions/blob/main/mix.exs#L35

@mpneuried
Copy link
Member Author

Looks good, a couple of things related to this PR:

The comment in this line should be changed: https://github.com/box-id/workflow_engine/blob/main/lib/actions/action.ex#L4

Good catch

It would be nice to provide the actual github ssh links here, so that we can just copy them over: https://github.com/box-id/workflow_engine_actions/blob/main/README.md?plain=1#L25

The variant in the Readme should work now, until the workflow_engine PR is done and the tag 2.0.0 is created.
In the workflow_engine_actions it's prepared in the mix.ex

Do the actions need to be nested in the lib/actions? They could just live in the lib IMO https://github.com/box-id/workflow_engine_actions/tree/main/lib/actions

I'm fine with both, so I removed the subfolder.

Do we use this anywhere? https://github.com/box-id/workflow_engine_actions/blob/main/mix.exs#L35

I already found this before, but it wasn't yet pushed ;-)

@mpneuried mpneuried merged commit 3fd81a8 into main Jun 18, 2025
11 checks passed
@mpneuried mpneuried deleted the PD-3318-extract-boxid-actions branch June 18, 2025 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants