-
Notifications
You must be signed in to change notification settings - Fork 148
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
Refactor LFS file handling in CI slightly #833
Conversation
This ensures: 1) The hashing differences on windows and linux don't impact us - https://xi.zulipchat.com/#narrow/channel/419691-linebender/topic/Parley.20CI.20failing.20.28due.20to.20LFS.20failure.3F.29/near/500383610 2) Only explicitly asked for LFS files are downloaded (avoiding accidental successes if these get out of sync) 3) There's a single source of truth for which files are impacted
This is a bit awkward
@@ -15,6 +15,8 @@ env: | |||
# List of packages that can not target Wasm. | |||
# `vello_tests` uses `nv-flip`, which doesn't support Wasm. | |||
NO_WASM_PKGS: "--exclude vello_tests --exclude simple_sdl2" | |||
# The files stored in LFS the tests need to access | |||
LFS_FILES: "vello_tests/snapshots/*.png" |
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.
I'm not sure how well this will handle adding additional files.
I think the approach is to add a comma within the quotes, which I think might work in both places. But it's a bit unclear
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.
We can cross that bridge when we actually need to. It's just a question of style - there are multiple possible solutions. 👍
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.
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.
The issue is isomorphic to https://github.com/orgs/community/discussions/25761#discussioncomment-6508758, I believe
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.
Aha, that soon. I can look at it later today and/or tomorrow if you guys haven't solved it before then.
This ensures: