Skip to content

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

Merged
polac24 merged 1 commit into
spotify:masterfrom
CharlieSuP1:master
Jul 10, 2024
Merged

prebuild should run artifactsOrganizer.prepare(artifact:) method even if artifacts exists locally#227
polac24 merged 1 commit into
spotify:masterfrom
CharlieSuP1:master

Conversation

@CharlieSuP1
Copy link
Copy Markdown

No description provided.

@polac24
Copy link
Copy Markdown
Collaborator

polac24 commented Sep 20, 2023

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
Copy link
Copy Markdown
Author

CharlieSuP1 commented Sep 20, 2023

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?

@CharlieSuP1 CharlieSuP1 reopened this Sep 20, 2023
@polac24
Copy link
Copy Markdown
Collaborator

polac24 commented Sep 20, 2023

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

@polac24 polac24 merged commit 75bac32 into spotify:master Jul 10, 2024
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.

2 participants