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

remove unused dependencies & add GHA flow #429

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

ithinuel
Copy link
Member

@ithinuel ithinuel commented Aug 23, 2022

Should it be made part of the existing jobs or parallel ones?

@ithinuel ithinuel force-pushed the remove-unused-dependencies branch 5 times, most recently from 7693758 to d18b01c Compare August 23, 2022 16:04
@ithinuel ithinuel marked this pull request as ready for review August 23, 2022 16:21
@ithinuel
Copy link
Member Author

Either way, it make the checks much much longer, so I'm not sure it's entirely worth it to have it as workflow.
We might just be good having it run manually every now and then.

@ithinuel ithinuel requested review from jannic and 9names August 23, 2022 16:22
Copy link
Member

@9names 9names left a comment

Choose a reason for hiding this comment

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

Glad to drop all the unused deps.
Looks like all the extra time is just building cargo-udeps.
I couldn't manage to get toolchain caching working correctly, but it should be possible to do this once and cache it for subsequent runs...

@ithinuel
Copy link
Member Author

Seems like the use-tool-cache option is deprecated ( https://github.com/actions-rs/tool-cache/ is archived )
And support using @action/cache has been waiting for about 2years: actions-rs/core#125 and actions-rs/install#5

@9names
Copy link
Member

9names commented Aug 25, 2022

Ah. Maybe using a prebuilt version instead? There are binary releases on github (need to untar them though).
I'll check how fast that is...

@9names
Copy link
Member

9names commented Aug 25, 2022

takes 1.5 seconds on my machine.
wish i didn't have to encode the version number in the string, maybe we could regex it out somehow but it might be more cursed...

wget -q -O - https://github.com/est31/cargo-udeps/releases/download/v0.1.32/cargo-udeps-v0.1.32-x86_64-unknown-linux-gnu.tar.gz | tar -xz && mv ./cargo-udeps-v0.1.32-x86_64-unknown-linux-gnu/cargo-udeps ~/.cargo/bin && rm ./cargo-udeps-v0.1.32-x86_64-unknown-linux-gnu -r

@ithinuel ithinuel force-pushed the remove-unused-dependencies branch 3 times, most recently from f94b267 to ce6d0e9 Compare August 26, 2022 01:55
@ithinuel
Copy link
Member Author

I found https://github.com/aig787/cargo-udeps-action :D seems to work as expected

@ithinuel ithinuel merged commit 13cad64 into rp-rs:main Aug 26, 2022
@ithinuel ithinuel deleted the remove-unused-dependencies branch August 26, 2022 02:42
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