Skip to content

Conversation

@Davidonium
Copy link

@Davidonium Davidonium commented Jul 15, 2025

I tried to do some improvements that were implemented in v2 in v1. I made those that I believe don't need a major version bump. Here's a list of my changes:

  • Detect circular dependencies.
  • Use the reflect package directly instead of fmt.Sprintf("%T") to avoid parsing code for generating service names.
  • ShutdownContext in the injector calls ShutdownContext on services that implement it.
  • Use Fully Qualified Service Names if a flag is set to true to maintain old behavior but be able to opt in to the safe one.
  • Change the ListInvokedServices method to return always the same list, previously it was iterating a map. Thus the order was random every time (tests weren't showing this because they iterate over a very little list).

I am opening this more as a point of discussion rather than seeking it to be merged. I believe these points can be achieved without a v2 version bump, allowing users to get features without package migrations.

If any of these seem interesting to actually be merged I can split them up in different non draft PRs for you to make it easier to review and merge.

Let me know what you think.

Thanks for the hard work you put into this library.

@samber samber changed the base branch from master to v1 September 26, 2025 12:02
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.

1 participant