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

Adds imported ZON files to the cache manifest #22750

Closed
wants to merge 2 commits into from

Conversation

MasonRemaley
Copy link
Contributor

@MasonRemaley MasonRemaley commented Feb 4, 2025

fixes #22746

Someone more familiar with the way caching works than me should probably take a quick look at this before it's merged, I only just looked at this part of the codebase for the first time while writing this.

In particular:

  1. I'm using addFilePost. I picked this because it's the most similar to what embed file does but doesn't require the file contents, but maybe it's not the right option.

  2. I'm unsure about the way I handle the errors that addFilePost can return. Other places in this file seem to have a TODO and just call sema.fail in similar circumstances, so I've done that here, but I feel a little bad proliferating the TODO without understanding it.

@mlugg
Copy link
Member

mlugg commented Feb 4, 2025

Before merging this, I'm going to see if I can get my PR up and passing instead, since it will tie ZON in with the pipeline more correctly (whereas here it's kinda hacked into Sema). If I can't do that, I'll merge this (barring any CI failures) to fix the immediate major bug.

@mlugg
Copy link
Member

mlugg commented Feb 4, 2025

Hopefully superseded by #22754.

@alexrp alexrp closed this Feb 5, 2025
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.

@import ZON not picking up changed files
3 participants