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

Base DependencyInjection setup #11

Merged
merged 2 commits into from
Jul 12, 2023
Merged

Conversation

G8XSU
Copy link
Collaborator

@G8XSU G8XSU commented May 12, 2023

Depends on #10

  • Add Base DI setup
  • Use guice as DI using hk2-guice bridge
  • Bind PostgresKVStore as default KVStore
  • Add HikariCp connection pooling for jdbc

@G8XSU G8XSU mentioned this pull request May 12, 2023
31 tasks
@G8XSU G8XSU requested a review from jkczyz July 12, 2023 08:13
@G8XSU G8XSU marked this pull request as ready for review July 12, 2023 08:14
Copy link

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

Could you add a bit more to the PR description and commit messages. Doesn't have to be lengthy but including the what and why for full context. Typical guide for this is https://cbea.ms/git-commit/.

Comment on lines +23 to 30
private void initGuiceIntoHK2Bridge(ServiceLocator serviceLocator, Injector injector) {
GuiceBridge.getGuiceBridge().initializeGuiceBridge(serviceLocator);
GuiceIntoHK2Bridge guiceBridge = serviceLocator.getService(GuiceIntoHK2Bridge.class);
guiceBridge.bridgeGuiceInjector(injector);
}
Copy link

Choose a reason for hiding this comment

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

Having no idea how this works, could you give a TL; DR context in the commit message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think in this particular case, it would add value to include explaination-docs in code as well, working on adding it.

@G8XSU G8XSU requested a review from jkczyz July 12, 2023 18:27
Copy link

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

Thanks for adding context!

@G8XSU G8XSU merged commit 909f7cc into lightningdevkit:main Jul 12, 2023
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