Skip to content

Create ContainerManager to prevent needing to directly check for Container registrations#279

Merged
skorulis-ap merged 1 commit intomainfrom
skorulis/container-manager
May 8, 2025
Merged

Create ContainerManager to prevent needing to directly check for Container registrations#279
skorulis-ap merged 1 commit intomainfrom
skorulis/container-manager

Conversation

@skorulis-ap
Copy link
Copy Markdown
Contributor

@skorulis-ap skorulis-ap commented May 5, 2025

This change is to prevent the following errors coming up during testing. The containers get automatically created but since it checks the resolver first an error is raised in the logs which increases noise when looking for real issues.

Swinject: Resolution failed. Expected registration:
  { Service: Container<AccountResolver>, Factory: Resolver -> Container<AccountResolver> }

@skorulis-ap skorulis-ap marked this pull request as ready for review May 6, 2025 22:12
@skorulis-ap skorulis-ap requested a review from bradfol May 6, 2025 22:12
Copy link
Copy Markdown
Contributor

@bradfol bradfol left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment thread Sources/Knit/Container.swift Outdated
Comment on lines +65 to +66
// ContainerManager from the parent of the SwinjectContainer
private let parent: ContainerManager?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alternately we could get this though the swinjectContainer's parent

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The Swinject.Container parent is private.

@skorulis-ap skorulis-ap force-pushed the skorulis/container-manager branch from 8052d30 to d7c19ff Compare May 7, 2025 07:13
@skorulis-ap skorulis-ap force-pushed the skorulis/container-manager branch from d7c19ff to 2af9fb6 Compare May 7, 2025 08:30
@skorulis-ap skorulis-ap merged commit f01febf into main May 8, 2025
2 checks passed
@skorulis-ap skorulis-ap deleted the skorulis/container-manager branch May 8, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants