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.

Sign up for GitHub

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

Jump to bottom

Add new providers to jOOQ auto-configuration #24485

Closed
wants to merge 1 commit into from
Closed

Add new providers to jOOQ auto-configuration #24485

wants to merge 1 commit into from

Conversation

vkuzel
Copy link

@vkuzel vkuzel commented Dec 13, 2020

jOOQ introduced following new providers. Auto-configuration should be able to pick them up a and put into jOOQ configuration. This pull request makes it happen.

New providers added to JooqAutoConfiguration:

  • DiagnosticsListenerProvider
  • UnwrapperProvider
  • CharsetProvider
  • ConverterProvider

Thank you for reviewing this PR.

@vkuzel
Add new providers to jOOQ auto-configuration
011571e 
New providers:
* `DiagnosticsListenerProvider`
* `UnwrapperProvider`
* `CharsetProvider`
* `ConverterProvider`
@pivotal-issuemaster
Copy link

@vkuzel Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 13, 2020
@pivotal-issuemaster
Copy link

@vkuzel Thank you for signing the Contributor License Agreement!

@lukaseder
Copy link
Contributor

I wonder if this discovery could be automated. All of these auto-configuration candidates are always located in org.jooq.Configuration...

@snicoll
Copy link
Member

snicoll commented Dec 14, 2020 •
edited
Loading

Given the number of callbacks I am also considering a different approach. Perhaps a DefaultConfigurationCustomizer would be a better fit as I don't think these need to be exposed as beans for other purposes. If so, it feels a bit odd to me that we'd have to register so many beans potentially just for the purpose of the auto-configuration to find them.

Flagging for team attention to see what the rest of the team thinks.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Dec 14, 2020
@chrisrhut chrisrhut mentioned this pull request Jan 2, 2021
Closed
@snicoll
Copy link
Member

snicoll commented Jan 11, 2021

@vkuzel thank you for the PR and for letting us know about those new callbacks in jOOQ. We've discussed this and decided to fix this in a different way (#24732). Thanks again.

@snicoll snicoll closed this Jan 11, 2021
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jan 11, 2021
@snicoll snicoll removed their assignment Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vkuzel @pivotal-issuemaster @lukaseder @snicoll @spring-projects-issues