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

Issue 3469 dependency transition 13 #3572

Merged
merged 8 commits into from
Jan 1, 2025

Conversation

finestructure
Copy link
Member

@finestructure finestructure commented Dec 30, 2024

This is the first step to move the Github related dependencies into a GithubClient DependencyClient.

This step moves fetchLicense and also drops the client parameter from its function call. This trickles through to a few functions in Github which now have client-free overloads. The outdated versions are marked with deprecation warnings. These will all go away once we've fully transition alle three Github dependency functions over to GithubClient.

@cla-bot cla-bot bot added the cla-signed label Dec 30, 2024
@finestructure finestructure marked this pull request as draft December 30, 2024 14:38
@finestructure
Copy link
Member Author

I've marked this WIP for now as I've encountered a potentially blocking issue for using typed throws with swift-dependencies. If that can't be resolved, we may want to hold off moving Github functions out of AppEnvironment so we don't carry around deprecation warnings and method overloads for a long time.

@finestructure
Copy link
Member Author

finestructure commented Jan 1, 2025

I've opened a discussion about the issue here: pointfreeco/swift-dependencies#324

There are a couple of possible workarounds:

  • isolate the typed throw closures to a single @DependencyClient (possibly requiring one for each) - not ideal
  • do this without the @DependencyClient macro (haven't tried it yet but that should work)

@finestructure
Copy link
Member Author

Dropping the @DependencyClient macro worked for the typed throws dependency as a workaround.

@finestructure finestructure marked this pull request as ready for review January 1, 2025 12:15
@finestructure finestructure disabled auto-merge January 1, 2025 12:27
@finestructure finestructure merged commit c61c582 into main Jan 1, 2025
6 checks passed
@finestructure finestructure deleted the issue-3469-dependency-transition-13 branch January 1, 2025 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants