spotify / XCRemoteCache

Other
825 stars 50 forks source link

prebuild should run artifactsOrganizer.prepare(artifact:) method even if artifacts exists locally #227

Open CharlieSuP1 opened 9 months ago

polac24 commented 9 months ago

Curious, in what scenario is required? If a previous build was interrupted in the middle and left unprocessed and unzipped file?

The change looks legit - can you write a test for that?

CharlieSuP1 commented 9 months ago

Yes, I can write a test for that. But before that I want to discuss the logic to see if I miss anything? image In my case, before the change I submitted, whenever code entered case .artifactExists(let artifactDir): (line 92) branch, line 104 would throw an ArtifactMetaUpdaterError.artifactLocationIsUnknown error. Is that designed so?

polac24 commented 9 months ago

You are correct, not calling prepare was a bug, introduced in #214. Thanks for fixing it.