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

polac24 commented Sep 20, 2023

Copy link
Copy Markdown
Collaborator

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

CharlieSuP1 commented Sep 20, 2023

Copy link
Copy Markdown
Author

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

polac24 commented Sep 20, 2023

Copy link
Copy Markdown
Collaborator

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