-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Fix build break and build release #2242
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but I have some questions.
profile: dev | ||
- runner: macos-14 | ||
target: aarch64-apple-darwin | ||
profile: release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this one build the fastest? And can you add a comment here because it's a bit subtle since it's in the middle of the list?
Arguably we should do one Mac and one Linux release
mode because there is stuff conditionally compiled for Landlock that sometimes I forget to test...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added linux
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both release builds are slow so I kept them non-blocking for merge.
@@ -77,7 +87,7 @@ jobs: | |||
~/.cargo/registry/cache/ | |||
~/.cargo/git/db/ | |||
${{ github.workspace }}/codex-rs/target/ | |||
key: cargo-${{ matrix.runner }}-${{ matrix.target }}-${{ hashFiles('**/Cargo.lock') }} | |||
key: cargo-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can/should we update rust-release.yml
to match and get cache hits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
…key for release builds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding a better way to keep the tree green!
Build release profile for one configuration.