Skip to content
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

[ABW-4014] Bump TCA to 1.17.0 #1423

Merged
merged 15 commits into from
Dec 18, 2024
Merged

[ABW-4014] Bump TCA to 1.17.0 #1423

merged 15 commits into from
Dec 18, 2024

Conversation

danvleju-rdx
Copy link
Contributor

@danvleju-rdx danvleju-rdx commented Dec 16, 2024

Jira ticket: ABW-4014

Description

  • Bumped TCA to 1.17.0, along with all related dependencies.
  • unimplemented now requires a placeholder parameter for cases where the placeholder cannot be generated.
  • swiftui-navigation has been renamed to swift-navigation.
  • Applied changes from the unmerged Simplify dependencies #1298

@danvleju-rdx danvleju-rdx marked this pull request as draft December 16, 2024 14:51
@danvleju-rdx danvleju-rdx marked this pull request as ready for review December 17, 2024 08:47
@@ -27,12 +27,12 @@ extension CloudBackupClient: TestDependencyKey {
)

static let testValue = Self(
isCloudProfileSyncEnabled: unimplemented("\(Self.self).isCloudProfileSyncEnabled"),
isCloudProfileSyncEnabled: noop.isCloudProfileSyncEnabled,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use unimplemented(description:placeholder:)? Same for other occurrences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the tests where these clients are used fail when unimplemented is used

Copy link
Contributor

Choose a reason for hiding this comment

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

Start the fail with the update to 1.17.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can use Sargon .sample values for noop in most clients? The fact that we throw is just because we did not have sample value for most types before Sargon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s a good idea, but it’ll take some time to go through all the noops, so I’m leaving it as a future improvement.

@@ -12,6 +12,6 @@ extension LocalAuthenticationClient: TestDependencyKey {
queryConfig: { .biometricsAndPasscodeSetUp },
authenticateWithBiometrics: { true },
setAuthenticatedSuccessfully: unimplemented("\(Self.self).setAuthenticatedSuccessfully"),
authenticatedSuccessfully: unimplemented("\(Self.self).authenticatedSuccessfully")
authenticatedSuccessfully: unimplemented("\(Self.self).authenticatedSuccessfully", placeholder: AsyncLazySequence([]).eraseToAnyAsyncSequence())
Copy link
Contributor

Choose a reason for hiding this comment

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

better to define noop to follow the same pattern

@danvleju-rdx danvleju-rdx merged commit 49aa660 into main Dec 18, 2024
6 checks passed
@danvleju-rdx danvleju-rdx deleted the ABW-4014-bump-tca branch December 18, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants