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

validate cache writes are successful #8727

Closed

Conversation

kkmuffme
Copy link
Contributor

No description provided.

@kkmuffme kkmuffme force-pushed the validate-cache-writes-succeeded branch 2 times, most recently from 62e20f1 to eff7f92 Compare November 21, 2022 10:46
@kkmuffme kkmuffme force-pushed the validate-cache-writes-succeeded branch from eff7f92 to 544d968 Compare November 21, 2022 10:49
@kkmuffme kkmuffme marked this pull request as ready for review November 21, 2022 11:06
@weirdan weirdan added the release:internal The PR will be included in 'Internal changes' section of the release notes label Nov 23, 2022
@weirdan
Copy link
Collaborator

weirdan commented Nov 23, 2022

It would be nice to hide all this heavy FS wizardry behind a simpler interface. E.g.

interface CacheBlob {
   public function read(): string;
   public function write(string): bool;
}

final class CacheFile implements CacheBlob {
   private string $filename;
   public function read(): string {
      // lock the file
      // read
      // validate that it's fully read
      // return 
   }

   public function write(string $data): bool {
      // write to a temporary file
      // validate it's fully written
      // move it to $this->filename
   }
}

@kkmuffme
Copy link
Contributor Author

Definitely, I just don't have time right now (this PR was a replace all regex for most parts). Will get back to it at some point and improve it

@orklah orklah marked this pull request as draft February 6, 2023 20:39
@kkmuffme kkmuffme closed this Jun 13, 2023
kkmuffme added a commit to kkmuffme/psalm that referenced this pull request Jun 13, 2023
kkmuffme added a commit to kkmuffme/psalm that referenced this pull request Jun 13, 2023
followup to vimeo#8727

(cherry picked from commit d1cc241)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:internal The PR will be included in 'Internal changes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants