-
Notifications
You must be signed in to change notification settings - Fork 10
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
build: improved CI and cleanup #254
build: improved CI and cleanup #254
Conversation
acd0970
to
7d9309f
Compare
Codecov Report
|
7d9309f
to
9f9cfcc
Compare
9f9cfcc
to
e28b82b
Compare
Signed-off-by: blu3beri <[email protected]>
e28b82b
to
7c2e6cc
Compare
uses: Swatinem/rust-cache@v2 | ||
with: | ||
shared-key: deps | ||
cache-on-failure: true |
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.
why cahce the failed resources?
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.
Mainly taken this from a rust CI under Hyperlegder, but if we fetch 80% of deps and fail we still whould like to cache 80% of the deps. I actually will check later on if caching is setup properly inside the CI though.
uses: Swatinem/rust-cache@v2 | ||
with: | ||
shared-key: deps | ||
cache-on-failure: true |
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.
same as above
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.
Same as above.
uses: codecov/codecov-action@v3 | ||
with: | ||
token: ${{ secrets.CODECOV_TOKEN }} | ||
fail_ci_if_error: true |
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.
interesting that this doesn't fail w/o this flag provided
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.
Uploading coverage can fail if their site or anything is down which might make it annoying. I would like to be strict here and only pass if everything works. Not opposed to removing it though.
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.
No, I see your reasoning. I meant more as in I would've thought initially that if error occurs it fail w/o having to explicitly set this
@@ -29,6 +29,6 @@ pub async fn parse_basic_message_args( | |||
}; | |||
agent.send_message(send_options).await.map(|_| { | |||
loader.stop(); | |||
log!("Successfully sent message") | |||
log!("Successfully sent message"); |
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.
makes sense
Signed-off-by: blu3beri [email protected]