Modernize app example #231
Conversation
|
Can one of the admins verify this patch? |
6 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
stevapple
force-pushed
the
update-app
branch
2 times, most recently
from
8a1e613
to
0ebb9dd
Compare
tomerd
added
the
patch-version-bump-only
|
thank @stevapple ! lets get #228 in first |
|
@swift-server-bot test this please |
...calDebugging/MyApp/MyApp.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist
Outdated
Show resolved
Hide resolved
Examples/LocalDebugging/MyApp/MyApp.xcodeproj/project.xcworkspace/contents.xcworkspacedata
Outdated
Show resolved
Hide resolved
stevapple
force-pushed
the
update-app
branch
from
0ebb9dd
to
61a8a2d
Compare
stevapple
force-pushed
the
update-app
branch
from
61a8a2d
to
dc491d5
Compare
|
I've rebased the branch against #228, so there should be no pain to merge. |
|
@swift-server-bot test this please |
stevapple
force-pushed
the
update-app
branch
from
57ba0f5
to
6bf66f6
Compare
stevapple
force-pushed
the
update-app
branch
from
6bf66f6
to
2b48c13
Compare
|
@swift-server-bot test this please |
| @@ -58,22 +67,16 @@ struct ContentView: View { | |||
| let (data, response) = try await URLSession.shared.data(for: request) | |||
|
|
|||
| guard let httpResponse = response as? HTTPURLResponse else { | |||
| throw CommunicationError(reason: "invalid response, expected HTTPURLResponse") | |||
| return "invalid response, expected HTTPURLResponse" | |||
There was a problem hiding this comment.
I'm personally not a big fan of removing the CommunicationError here, since it hides what actually happens. We need to have some do {} catch {} somewhere else. This method should be async throws -> String.
There was a problem hiding this comment.
Uh yes.
I changed it into () async -> String because we need to ensure the label is changed in every path. throws could be preserved here and it doesn't affect.
There was a problem hiding this comment.
I would prefer something like this:
func register() await -> String {
do {
return try await self.runNetworkRegister(name: self.name, password: self.password)
} catch as CommunicationError {
return error.reason
} catch {
return error.localizedDescription
}
}There was a problem hiding this comment.
I would rather suggest we catch in the button action? The register() API is expected not to confuse an actual response and a thrown Error. runNetworkRegister(name:password:) looks redundant here.
There was a problem hiding this comment.
Conditional catch can also be eliminated here by implementing LocalizedError.
stevapple
force-pushed
the
update-app
branch
from
f39d76f
to
3c8345f
Compare
|
@tomerd ping |
|
@swift-server-bot test this please |
|
Xcode 13.2 and Swift Playgrounds 4 supports a new type of package-based app project ( I’m wondering if such change is too aggressive as most of us still use |
|
ping @tomerd @fabianfett |
Update
LocalDebuggingexample to reflect the latest SwiftUI practice.Motivation:
Since we already required Swift 5.5 with Concurrency (Xcode 13), it's natural to update the app part to reflect latest SwiftUI changes, which makes the code neater and more expressive.
Modifications:
LocalDebugging/MyAppto use SwiftUI lifecycle;LocalDebugging/MyAppto reflect SwiftUI best practice;LocalDebugging/Sharedto use Swift 5.5.Result:
A better demo app.