Skip to content

Conversation

@muir
Copy link
Owner

@muir muir commented Apr 10, 2025

Now that there is a v2, there is a problem with combining with v1.
Fail when there is a mismatch of versions.

@muir muir requested a review from Copilot April 10, 2025 18:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@muir muir requested a review from Copilot April 10, 2025 18:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

nject.go:147

  • Returning a provider instance with a fatal error instead of panicking changes the failure handling behavior. Please ensure that the rest of the code properly checks and handles the fatal error field to avoid silent failures.
return &provider{ ... fatal:  fmt.Errorf("cannot turn Collection into a function"), ... }

Comment on lines +310 to +311
switch fmt.Sprintf("%T", fn) {
case "nject.Collection", "*nject.Collection", "nject.provider", "*nject.provider":
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

Consider using a type assertion or a type switch directly on fn instead of comparing its type as a formatted string. This would be a more robust and type-safe approach to detect multiple versions of nject.

Suggested change
switch fmt.Sprintf("%T", fn) {
case "nject.Collection", "*nject.Collection", "nject.provider", "*nject.provider":
switch fn.(type) {
case *Collection, Collection, *provider, provider:

Copilot uses AI. Check for mistakes.
@muir muir merged commit c620a9b into main Apr 10, 2025
17 checks passed
@muir muir deleted the multiVersions branch April 10, 2025 19:01
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