-
Notifications
You must be signed in to change notification settings - Fork 84
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
Improve CI #296
Improve CI #296
Conversation
This is fine because `master` branch now is protected by merge queue
@@ -111,8 +111,10 @@ store-success-output = false | |||
store-failure-output = true | |||
|
|||
[[profile.default.overrides]] | |||
filter = 'binary(=hyper)' | |||
retries = { backoff = "fixed", count = 6, delay = "1s" } | |||
# `hyper` test targeting external Google's service, it would sometomes failed because of rate limit |
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.
Unit tests really shouldn't rely on external services, especially ones that might rate limit us. Can this test be changed so it runs its own server locally that the test can make requests to? Ideally, the server should use a different network implementation, so something like python.
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.
Make sense.
Created an issue for this: #297
.github/workflows/publish-crates.yml
Outdated
echo "Publishing crate: $crate_name" | ||
cargo publish --locked --token ${CARGO_REGISTRY_TOKEN} --package "$crate_name" | ||
# Create a symbolic link to ensure correct usage of libclang | ||
sudo ln -s /lib/x86_64-linux-gnu/libclang-11.so.1 /lib/x86_64-linux-gnu/libclang.so |
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.
Since the .so needs to be there, just possibly not in the location where it's being looked for, can this be fixed by adding the appropriate directory to the search path for the linker and/or setting LD_LIBRARY_PATH when whatever needs this library is being run? Adding symlinks to standard system locations seems like not a good idea for a build/test system to be doing.
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.
removed
I see, after recheck. These lines seems not necessary.
It's not necessary now.
cargo-cache
fortest
workflow.