Consolidate CI hooks by EzraBrooks · Pull Request #299 · open-planning/roboplan

EzraBrooks · 2026-09-03T16:24:49Z

By invoking format and build_pypi_wheels from workflow_call, we ensure that our CI trigger and schedule strategy does not drift from job to job.

EzraBrooks · 2026-09-03T16:25:15Z

This might be considered a matter of taste - but personally I like having all my CI jobs visible in a singular workflow in Actions.

EzraBrooks · 2026-09-03T16:30:46Z

Ah, because of the way the job names are generated (pre-pending their caller and /) this will also require changing the required job names in GitHub. So if we like both this and #296 we probably should merge as one

sea-bass

I like this as well, so we can merge the 2 PRs as one. I can go into the admin settings and change the required checks to pass after the fact.

Let me know when/if you want to consolidate these 2 PRs and we shall get it through.

EzraBrooks · 2026-09-03T23:21:11Z

Would it make sense to also move the current jobs that live at the top level of build_and_test.yml into their own reusable workflows, just to get ahead of any future naming problems? That way the whole workflow view in GitHub will already be folderized into format, wheels, Pixi, and ROS builds.

I could go either way personally, just thinking about how to prevent more faffing about with GitHub settings.

sea-bass · 2026-09-03T23:48:02Z

That seems like a good idea for consistency!

EzraBrooks · 2026-09-04T15:30:36Z

@@ -1,4 +1,4 @@
-name: build_and_test
+name: CI


Changed this name while we're gonna be messing with names anyway - since this workflow just orchestrates "CI" in general.

By invoking format and build_pypi_wheels from workflow_call, we ensure that our CI trigger and schedule strategy does not drift from job to job.

EzraBrooks · 2026-09-04T15:35:13Z

we can do some more cosmetic tweaking to the workflow names and job names once we run and see how they look in practice. they might be a bit verbose right now

EzraBrooks · 2026-09-04T15:45:57Z

+      - name: Sanitize tag name
+        id: sanitize_tag
+        run: |
+          BRANCH="${{ github.head_ref || github.ref_name }}"
+          TAG="${BRANCH//\//-}"
+          echo "tag=$TAG" >> $GITHUB_OUTPUT


In a follow-up PR (since I don't want to risk functional changes in this PR that's mostly cosmetic), I think we should move this up to the root workflow and pass the tag name as an argument to the two jobs that use it.

Make the contents of the job clear in its name and remove inline YAML logic.

EzraBrooks

Thoughts on these proposed names for the folderization of the workflow?

EzraBrooks · 2026-09-04T19:09:19Z

I got a bit lost in the sauce on changing the names of things but I think it looks really nice now.

eholum

I like this change! Def easier to see the results. Minor nit in what looks like a bypassed test, but otherwise even the required status checks panel is nicer to look at:

eholum · 2026-09-04T20:25:20Z

    assert "workflow_call:" in workflow
+    assert "pull_request:" not in workflow
+    assert "push:" not in workflow
+    assert "workflow_dispatch:" not in workflow


Removed, right? This test now fails for me locally,

> assert "workflow_dispatch:" not in workflow E AssertionError: assert 'workflow_dispatch:' not in 'name: Build...und: error\n' E E 'workflow_dispatch:' is contained here: E name: Build PyPI wheels E E on: E workflow_dispatch: E workflow_call: {}... E E ...Full output truncated (133 lines hidden), use '-vv' to show

That said does this get run by anything?

Suggested change

assert "workflow_dispatch:" not in workflow

failing locally but not in CI? suspicious..

It appears this test has never been invoked by CI. It seems like it's mostly just AI ramblings. Should I just delete it?

A non-functional, non-executed test definitely doesn't add value... Seems like it was added in the initial wheels setup, #247.

So since we haven't been using it I would vote delete. But if someone who knows more about python packages (namely, everyone) than I do might see the value in actually wiring it to something...

I actually have an incoming draft that unifies the wheel build into the new superbuild. Also, the CI job for building the wheels already sanity-checks them by importing them

eholum

LGTM, can coordinate with @sea-bass about merging and updating the status checks...

sea-bass

Updated naming LGTM, gonna fiddle with the required checks now

sea-bass reviewed Sep 3, 2026

View reviewed changes

EzraBrooks force-pushed the consolidate-ci-hooks branch from 42a2879 to 40522fb Compare September 4, 2026 15:22

EzraBrooks mentioned this pull request Sep 4, 2026

ci: change Python stub generation from imperative to declarative #296

Closed

EzraBrooks commented Sep 4, 2026

View reviewed changes

Consolidate CI hooks

5721b17

By invoking format and build_pypi_wheels from workflow_call, we ensure that our CI trigger and schedule strategy does not drift from job to job.

EzraBrooks force-pushed the consolidate-ci-hooks branch from 8ff35c2 to 8dc0f3a Compare September 4, 2026 15:32

EzraBrooks requested a review from sea-bass September 4, 2026 15:34

EzraBrooks commented Sep 4, 2026

View reviewed changes

EzraBrooks added 2 commits September 4, 2026 09:56

Hoist "generate python stubs" to the CI job matrix

942104b

Make the contents of the job clear in its name and remove inline YAML logic.

ci: only use top-level CI workflow for orchestration

897a7ce

EzraBrooks force-pushed the consolidate-ci-hooks branch from 8dc0f3a to 897a7ce Compare September 4, 2026 15:57

EzraBrooks commented Sep 4, 2026

View reviewed changes

EzraBrooks force-pushed the consolidate-ci-hooks branch 3 times, most recently from c180eb3 to 8ba816a Compare September 4, 2026 18:57

New names for workflow callsites

1761eed

EzraBrooks force-pushed the consolidate-ci-hooks branch from 8ba816a to 1761eed Compare September 4, 2026 18:58

eholum reviewed Sep 4, 2026

View reviewed changes

Remove unused test

6df1032

eholum approved these changes Sep 4, 2026

View reviewed changes

sea-bass approved these changes Sep 4, 2026

View reviewed changes

sea-bass merged commit 25c75a3 into open-planning:main Sep 4, 2026
21 checks passed

Conversation

EzraBrooks commented Sep 3, 2026

Uh oh!

EzraBrooks commented Sep 3, 2026

Uh oh!

EzraBrooks commented Sep 3, 2026

Uh oh!

sea-bass left a comment

Choose a reason for hiding this comment

Uh oh!

EzraBrooks commented Sep 3, 2026

Uh oh!

sea-bass commented Sep 3, 2026

Uh oh!

EzraBrooks Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

EzraBrooks commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EzraBrooks Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

EzraBrooks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EzraBrooks commented Sep 4, 2026

Uh oh!

eholum left a comment

Choose a reason for hiding this comment

Uh oh!

eholum Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

EzraBrooks Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

EzraBrooks Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

eholum Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

EzraBrooks Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

eholum left a comment

Choose a reason for hiding this comment

Uh oh!

sea-bass left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

EzraBrooks commented Sep 4, 2026 •

edited

Loading