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

Fix CI clippy check by removing caching #179

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

echeran
Copy link
Contributor

@echeran echeran commented Jul 17, 2020

This PR fixes a bug in the CI clippy action. Lost in the iterations on the original clippy PR (#161) was the fact that we don't want to include the cache step in the clippy action since it restores code from cache that subsequently gets picked up by cargo.

I did some local testing, then verified by testing using a draft PR (#178) to test for real, and the fix works to address the original problem of clippy errors on non-existent code.

What should remain in the clippy output for the PR in question (#141) after this fix are the real clippy errors on the actual existing code of that PR.

@echeran echeran requested a review from sffc July 17, 2020 19:08
@echeran echeran requested a review from a team as a code owner July 17, 2020 19:08
@coveralls
Copy link

Pull Request Test Coverage Report for Build 28d04d0de48bd3f9151ce26560a8e5f6c754bee2-PR-179

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 87.199%

Totals Coverage Status
Change from base Build 461bf8eb055010ac96c473712ba46657faebf981: 0.0%
Covered Lines: 688
Relevant Lines: 789

💛 - Coveralls

@sffc
Copy link
Member

sffc commented Jul 17, 2020

Should we cache the "registry" and "index", and only un-cache the "build"?

@echeran
Copy link
Contributor Author

echeran commented Jul 17, 2020

Even before the caching config was fixed, I don't recall the "registry" and "index" steps taking that long -- it was always the build step that took long because of the need to download and compile deps (example from before caching was fixed). They usually take 1s - 6s. I'm okay with not changing things to partially cache to keep things less complicated with little wall clock time impact.

Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

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

OK, do what you think is best. LGTM

@echeran echeran merged commit 819147a into unicode-org:master Jul 17, 2020
@echeran echeran deleted the ci-clippy-fix branch October 6, 2020 17:57
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.

3 participants