Skip to content

Conversation

@G8XSU
Copy link
Contributor

@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
Contributor

@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
Contributor

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
Contributor 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
Contributor

@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