Skip to content

Clean up some CI loose ends related to recent changes#2372

Merged
Eliah Kagan (EliahKagan) merged 2 commits intoGitoxideLabs:mainfrom
EliahKagan:loose-ends
Jan 12, 2026
Merged

Clean up some CI loose ends related to recent changes#2372
Eliah Kagan (EliahKagan) merged 2 commits intoGitoxideLabs:mainfrom
EliahKagan:loose-ends

Conversation

@EliahKagan
Copy link
Copy Markdown
Member

This makes some changes that build on, clarify, or are unblocked by recent changes.

  • c97ef4e
    Delist libz-rs-sys from "pure" builds' *-sys allowlist
    Relates to: #2370

  • 1c74b91
    Add missing shell: bash comments as well as a conceptual comment
    Relates to: #2340

See commit messages for full details.

Because we no longer depend on it since bc2f339 (GitoxideLabs#2370).

For context on the associated TODO this completes and removes, see:

- rust-lang/flate2-rs#520 (superseded)
- rust-lang/flate2-rs#524
- rust-lang/flate2-rs#525
- Add missing comments on step-level `shell: bash` that were
  introduced without them in GitoxideLabs#2340.

- Add a conceptual comment where step-level `shell: bash` is
  intentionally omitted for the main nextest step of the
  `test-fast` jobs.

These changes are as discussed in GitoxideLabs#2340 comments, in particular:
GitoxideLabs#2340 (comment)
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR cleans up CI configuration following recent dependency and toolchain changes. It removes an outdated allowlist entry for libz-rs-sys after upgrading to a newer flate2 version (PR #2370), and adds clarifying comments about shell usage in various CI jobs following the migration from dtolnay/rust-toolchain to manual rustup commands (PR #2340).

Changes:

  • Removed libz-rs-sys from the *-sys dependencies allowlist in the pure build check
  • Added explanatory comments for shell: bash directives across multiple CI jobs
  • Added a detailed comment explaining why one test step deliberately uses different shells on different OSes

@EliahKagan Eliah Kagan (EliahKagan) marked this pull request as ready for review January 12, 2026 18:10
@EliahKagan
Copy link
Copy Markdown
Member Author

Eliah Kagan (EliahKagan) commented Jan 12, 2026

I'm curious if a Copilot review in #2370 would've identified the ability to do the c97ef4e change at that time. I strongly doubt that it would, but I'm not certain it couldn't. I'll have Copilot review that merged PR to see. (I'm mentioning this here instead of there, becuase Copilot might read it there as part of doing the review.)

Edit: It didn't, but maybe we can make it do that; see #2370 (comment).

@EliahKagan Eliah Kagan (EliahKagan) merged commit 30e1989 into GitoxideLabs:main Jan 12, 2026
36 checks passed
@EliahKagan Eliah Kagan (EliahKagan) deleted the loose-ends branch January 12, 2026 18:26
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