search

est31 / cargo-udeps

Find unused dependencies in Cargo.toml
Other
1.76k stars 46 forks source link

Build release artifacts for more targets #166

Closed NobodyXu closed 1 year ago

NobodyXu commented 1 year ago

Build for:

This PR also enables features vendored-{openssl, libgit2} for cross compilation, including all linux targets and aarch64-apple-darwin.

Signed-off-by: Jiahao XU Jiahao_XU@outlook.com

est31 commented 1 year ago

Run workflow release on pull_request & push to master

That's creating a lot of additional artifacts. I think it only makes sense to publish any artifacts on pushes to master, and the published artifacts should ideally be limited to the musl builds only, as they use the vendored OS libraries.

I'm also not sure if it makes sense to build armv7 builds these days for a development tool. Can you remove them? We can still reconsider our decision once users of that platform show up.

NobodyXu commented 1 year ago

That's creating a lot of additional artifacts. I think it only makes sense to publish any artifacts on pushes to master, and the published artifacts should ideally be limited to the musl builds only, as they use the vendored OS libraries.

NOTE that the code does not upload the artifacts, it merely builds them to confirm that the ci configuration is correct and the code can be run on that targets.

est31 commented 1 year ago

@NobodyXu for musl binaries an upload would be useful, for any other binaries, I don't think it makes sense to have them run even for checking, as I believe it's better that CI runs fast than that it checks all possible configurations.

NobodyXu commented 1 year ago

@NobodyXu for musl binaries an upload would be useful, for any other binaries, I don't think it makes sense to have them run even for checking, as I believe it's better that CI runs fast than that it checks all possible configurations.

Sure, once the CI run succeeds I will change it back to only run on release. But having it runs on CI is especially useful for PRs like this, where the release CI is changed but it is hard to test.

@est31 Perhaps it would be a good idea to allow release CI to be triggered manually?

est31 commented 1 year ago

But having it runs on CI is especially useful for PRs like this, where the release CI is changed but it is hard to test.

Absolutely. In that case you can always enable it locally for your PR, then wait until that CI has succeeded, and then un-do the change before the PR is merged. That's a strategy I've been using for PRs to other repositories already in the past as well.

NobodyXu commented 1 year ago

@est31 This PR is now ready to merged. I have reverted the changes as you requested.

NobodyXu commented 1 year ago

@est31 I've submit all PRs

est31 commented 1 year ago

I will do a release in the next 24 hours when I have time

NobodyXu commented 1 year ago

Thanks! There's no need to rush, take your time and have a good day!