avaje / avaje-inject

Dependency injection via APT (source code generation) ala "Server-Side Dagger DI"
https://avaje.io/inject
Apache License 2.0
227 stars 21 forks source link

Validate Qualifiers at compile time #645

Closed SentryMan closed 1 month ago

SentryMan commented 1 month ago

It seems previously we only validated that all the bean types were in the scope at compile time without looking at the qualifiers.

resolves #644

rob-bygrave commented 1 month ago

We should try to keep a PR focused on it's change only and not include other unrelated changes into it. My instinct is that "fix event publishers not getting detected" and "refactor reading external modules" should have been in separate PRs?

One aspect of this is that the PR Title is important documentation, and the other is from a git history perspective we want a commit to be focused on it's change. Putting multiple things into a commit makes life harder for our future selves when we try to understand the interaction between features by looking at commit history.

SentryMan commented 1 month ago

My instinct is that "fix event publishers not getting detected" and "refactor reading external modules" should have been in separate PRs?

sure I can break the external one into it's own thing, but the event publisher bug only arises from the changes in this PR. I'm totally unable to replicate it when I try it on master.

rob-bygrave commented 1 month ago

but the event publisher bug only arises from the changes in this PR

Ok cool. Then lets keep that in as part of this PR.