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

ci: add per-crate checks via cargo-hack #4166

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Apr 5, 2024

Describe your changes

Suggested by @erwanor, based on the Astria CI config [0]. The goal is to ensure that our default feature sets actually work, by compiling each crate in the workspace individually.

[0] https://github.com/astriaorg/astria/blob/6cc8e2b828f8f5ee65e03c2b3383c4252b4c6b81/.github/workflows/test.yml#L64-L68

Issue ticket number and link

Checklist before requesting a review

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

    The changes to crate content are explicitly declaring which imports should be activated based on feature sets. The rest of the diff is CI-only logic.

@conorsch conorsch force-pushed the ci-compile-each-crate-separately branch from 4e02841 to 41dce1e Compare April 6, 2024 01:21
@cratelyn cratelyn added this to the Sprint 4 milestone Apr 8, 2024
@cratelyn cratelyn added the A-CI/CD Relates to continuous integration & deployment of Penumbra label Apr 8, 2024
@conorsch conorsch force-pushed the ci-compile-each-crate-separately branch from 08beb3b to b06bb6e Compare April 8, 2024 17:44
@conorsch conorsch force-pushed the ci-compile-each-crate-separately branch from b06bb6e to 29ae1dc Compare April 8, 2024 19:52
@@ -1,6 +1,7 @@
use anyhow::{Context, Result};
use async_trait::async_trait;
use cnidarium::StateWrite;
#[cfg(feature = "component")]
Copy link
Member

Choose a reason for hiding this comment

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

nice catch!

Suggested by @erwanor, based on the Astria CI config [0].
The goal is to ensure that our default feature sets actually work,
by compiling each crate in the workspace individually.

fix: feature-gating across the workspace
Satisifes the new CI check by massaging the feature sets.

[0] https://github.com/astriaorg/astria/blob/6cc8e2b828f8f5ee65e03c2b3383c4252b4c6b81/.github/workflows/test.yml#L64-L68
@conorsch conorsch force-pushed the ci-compile-each-crate-separately branch from 29ae1dc to 6a0c2bb Compare April 8, 2024 20:48
@conorsch
Copy link
Contributor Author

conorsch commented Apr 8, 2024

Running the new job with 8vcpus, which we can get away with because we use generous caching. After running the new CI job a few times, observed the following durations:

  • 22m36s (cold cache)
  • 7m56s (warm cache, rerun)
  • 6m1s (warm cache, rebased)
  • 4m1s (warm cache, rebased)

Let's keep an eye on it: we can always bump up the runner size if it's too slow.

@conorsch conorsch marked this pull request as ready for review April 8, 2024 21:04
@conorsch conorsch requested a review from cratelyn April 8, 2024 21:05
Copy link
Contributor

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

✨ lovely! spotted one comment nitpick, otherwise i am happy to see some extra assurance that individual crates build. 📦

@conorsch conorsch merged commit f7234c0 into main Apr 8, 2024
8 checks passed
@conorsch conorsch deleted the ci-compile-each-crate-separately branch April 8, 2024 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI/CD Relates to continuous integration & deployment of Penumbra
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants