SwiftTool: refine wording in func *UpdateRepository
#5836
Conversation
MaxDesiatov
requested review from
abertelrud,
neonichu,
tomerd and
elsh
as code owners
|
@swift-ci please smoke test |
Sources/Commands/SwiftTool.swift
Outdated
| @@ -117,11 +117,11 @@ private class ToolWorkspaceDelegate: WorkspaceDelegate { | |||
| } | |||
|
|
|||
| func willUpdateRepository(package: PackageIdentity, repository url: String) { | |||
| self.outputHandler("Updating \(url)", false) | |||
| self.outputHandler("Fetching updates from \(url)", false) | |||
There was a problem hiding this comment.
Maybe "Checking for updates from..." @neonichu @abertelrud opinions?
There was a problem hiding this comment.
This wording sounds good to me since we don't know whether there will be any. Ideally we should later add another callback for if there actually are any, but that's a later PR.
MaxDesiatov
force-pushed
the
maxd/fetching-updates
branch
from
c87bd55 to
07298d0
Compare
|
@swift-ci please smoke test |
| } | ||
|
|
||
| func didUpdateRepository(package: PackageIdentity, repository url: String, duration: DispatchTimeInterval) { | ||
| self.outputHandler("Updated \(url) (\(duration.descriptionInSeconds))", false) | ||
| self.outputHandler("Checked for updates from \(url) (\(duration.descriptionInSeconds))", false) |
There was a problem hiding this comment.
I think this should remain "Updated", as we may have updated. We could / should modify this delegate to include the modified: Bool argument, but not sure how easy or hard that would be
|
@MaxDesiatov do we want to push this one over the finish line? any open questions? |
|
Thanks for the ping, I'll try to address feedback on this tomorrow. |
|
@MaxDesiatov could you update this one? |
|
Sorry, as discussed offline there are a few more changes that can be done on this front. I should've moved it to a draft state. |
Motivation:
Currently, when fetching updates from a repository, a diagnostic message "Updating" is printed, which is not clear enough. It's not inherently obvious whether this update is local or remote, whether it requires networking access etc.
Modifications:
"Updating" is replaced with "Checking for updates from", to indicate that remote access is required for this repository update.
Result:
It's more clear what exactly SwiftPM is doing with the expanded diagnostics messages.