Skip to content

Call artifacts download delegate final method #3150

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

Merged

Conversation

krzyzanowskim
Copy link
Contributor

Call atrifacts download delegate final method only if we ever try to download an artifact.

Motivation:

WorkspaceDelegate.didDownloadBinaryArtifacts is called despite there's nothing to download, nor we even tried to download anything. Given the name "did download" it should be called only if there was a download (attempt).

Modifications:

Check with flag whether any download happened.

Copy link
Member

@federicobucchi federicobucchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please rewrite the commit message including artifacts instead of atrifacts ?

@krzyzanowskim krzyzanowskim changed the title Call atrifacts download delegate final method Call artifacts download delegate final method Dec 27, 2020
@tomerd tomerd self-assigned this Dec 27, 2020
@tomerd
Copy link
Contributor

tomerd commented Dec 27, 2020

@swift-ci please smoke test

@federicobucchi
Copy link
Member

@krzyzanowskim thanks, I meant the commit message, not only the title in the PR:

Screen Shot 2020-12-27 at 3 14 30 PM

@krzyzanowskim krzyzanowskim force-pushed the marcin/call-did-download-if-download branch from dd208e6 to 6cc01dd Compare December 27, 2020 23:32
@krzyzanowskim
Copy link
Contributor Author

@federicobucchi sure. pushed. back you you.

@tomerd tomerd assigned neonichu and unassigned tomerd Jan 2, 2021
Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only possible concern would be if any client relies on always getting this callback, but I don't know of any one that currently does.

@tomerd
Copy link
Contributor

tomerd commented Jan 5, 2021

@swift-ci please smoke test

@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Jan 5, 2021
@tomerd tomerd merged commit 92c0d9c into swiftlang:main Jan 5, 2021
@krzyzanowskim krzyzanowskim deleted the marcin/call-did-download-if-download branch January 5, 2021 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants