Skip to content

fix demo deps - #238

Merged
michaelkirk merged 7 commits into
georust:masterfrom
michaelkirk:mkirk/fix-demo-deps
Aug 13, 2026
Merged

fix demo deps#238
michaelkirk merged 7 commits into
georust:masterfrom
michaelkirk:mkirk/fix-demo-deps

Conversation

@michaelkirk

@michaelkirk michaelkirk commented Aug 12, 2026

Copy link
Copy Markdown
Member
  • I agree to follow the project's code of conduct.
  • I added an entry to rstar/CHANGELOG.md if knowledge of this change could be valuable to users.

Edit: note this is based on #232 so merge that first.

cargo test / cargo check of the workspace currently fails.

Potentially (?) controversial is adding cargo check (of the demo) to CI. It could be potentially annoying to have the demo block merging of other work. It's also annoying now that I can't run cargo check in the workspace because the demo is broken. 🤷 I'm open to omitting the second commit.

@adamreichold

Copy link
Copy Markdown
Member

Potentially (?) controversial is adding cargo check (of the demo) to CI. It could be potentially annoying to have the demo block merging of other work. It's also annoying now that I can't run cargo check in the workspace because the demo is broken. 🤷 I'm open to omitting the second commit.

I think we deliberately removed it as to not block things on it. I think a more refined position would be a separate job that builds but does not fail the workflow. This way, we notice something breaking early and automatically, but we still do not block anything on these optional components. WDYT?

@michaelkirk

Copy link
Copy Markdown
Member Author

I've never used these "non-blocking" warning jobs before. It seems kind of useless as is, because there's no indication it failed unless you manually go look at the annotations in https://github.com/georust/rstar/actions/runs/31719804060

Since you brought it up @adamreichold, is there a better way to do this?

@adamreichold

Copy link
Copy Markdown
Member

Since you brought it up @adamreichold, is there a better way to do this?

You added it as a continue-on-error: true step to the existing job. I was thinking of a separate job "workspace check" that just is not considered by the conclusion job and therefore would not fail the overall build. This should then be visible as a separate "check" in the PR overview.

@michaelkirk

Copy link
Copy Markdown
Member Author

Like this @adamreichold? Otherwise if you have a clear vision of how to do this, please just push something up.

Comment thread .github/workflows/test.yml Outdated

@adamreichold adamreichold left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed like this, just one nit on the check step.

@michaelkirk
michaelkirk added this pull request to the merge queue Aug 13, 2026
Merged via the queue into georust:master with commit afae2ae Aug 13, 2026
6 checks passed
@michaelkirk
michaelkirk deleted the mkirk/fix-demo-deps branch August 13, 2026 19:21
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