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

refactor: remove action runners from public dune #10794

Merged

Conversation

rgrinberg
Copy link
Member

Action runners in this dune are a rather old version that it is very much out of date with the state of the actual feature and has many bugs and limitations that need a lot of upstreaming to fix. In its current state, it does not provide any value to users of public dune.

If we ever want to upstream this feature, it will be easier to just do it from scratch anyway instead of resolving the conflicts against some old version. This is due to the fact that the action runner API with the engine is rather small and well defined.

Once we introduce a better abstraction for action execution (as we are indeed planning), it will be probably be even simpler to upstream action runners.

All in all, we gain nothing on either team by keeping this dead weight around.

cc @snowleopard

Signed-off-by: Rudi Grinberg me@rgrinberg.com

(Build_config.get ()).action_runners ()
|> Fiber.parallel_iter ~f:Action_runner.cancel_build
in
Scheduler.cancel_current_build ()
Copy link
Member Author

Choose a reason for hiding this comment

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

This bit is completely broken anyway

Action runners in this dune are a rather old version that it is very
much out of date with the state of the actual feature and has many bugs
and limitations that need a lot of upstreaming to fix. In its current
state, it does not provide any value to users of public dune.

If we ever want to upstream this feature, it will be easier to just do
it from scratch anyway instead of resolving the conflicts against some
old version. This is due to the fact that the action runner API with the
engine is rather small and well defined.

Once we introduce a better abstraction for action execution (as we are
indeed planning), it will be probably be even simpler to upstream action
runners.

All in all, we gain nothing on either team by keeping this dead weight
around.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

<!-- ps-id: b913f829-def9-4d41-90d6-07da8060063b -->
@rgrinberg rgrinberg force-pushed the ps/rr/refactor__remove_action_runners_from_public_dune branch from 1154bcd to d40c5aa Compare August 15, 2024 13:35
@rgrinberg rgrinberg merged commit 36b12e2 into main Aug 16, 2024
28 checks passed
@rgrinberg rgrinberg deleted the ps/rr/refactor__remove_action_runners_from_public_dune branch August 16, 2024 05:12
Copy link
Collaborator

@snowleopard snowleopard left a comment

Choose a reason for hiding this comment

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

Thanks!

anmonteiro pushed a commit to anmonteiro/dune that referenced this pull request Nov 17, 2024
Action runners in this dune are a rather old version that it is very
much out of date with the state of the actual feature and has many bugs
and limitations that need a lot of upstreaming to fix. In its current
state, it does not provide any value to users of public dune.

If we ever want to upstream this feature, it will be easier to just do
it from scratch anyway instead of resolving the conflicts against some
old version. This is due to the fact that the action runner API with the
engine is rather small and well defined.

Once we introduce a better abstraction for action execution (as we are
indeed planning), it will be probably be even simpler to upstream action
runners.

All in all, we gain nothing on either team by keeping this dead weight
around.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
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.

2 participants