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

Auto-configure jOOQ with TransactionListenerProvider #13331

Closed
wants to merge 2 commits into from

Conversation

nosan
Copy link
Contributor

@nosan nosan commented Jun 1, 2018

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 1, 2018
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 2, 2018
@snicoll snicoll added this to the Backlog milestone Jun 2, 2018
@snicoll snicoll changed the title Auto-configure jOOQ with TransactionListenerProvider. Auto-configure jOOQ with TransactionListenerProvider Jun 2, 2018
@snicoll snicoll modified the milestones: Backlog, 2.0.3 Jun 4, 2018
@snicoll snicoll self-assigned this Jun 4, 2018
Copy link
Member

@snicoll snicoll 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 the PR. See comment for more details.

public DslContextConfiguration(JooqProperties properties,
ConnectionProvider connectionProvider, DataSource dataSource,
ObjectProvider<TransactionProvider> transactionProvider,
ObjectProvider<RecordMapperProvider> recordMapperProvider,
ObjectProvider<RecordUnmapperProvider> recordUnmapperProvider,
ObjectProvider<Settings> settings,
ObjectProvider<RecordListenerProvider[]> recordListenerProviders,
ExecuteListenerProvider[] executeListenerProviders,
ObjectProvider<VisitListenerProvider[]> visitListenerProviders) {
ObjectProvider<ExecuteListenerProvider[]> executeListenerProviders,
Copy link
Member

Choose a reason for hiding this comment

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

That change in ExecuteListenerProvider is unrelated to the purpose of this PR. Please revert it so that this change is properly focused.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jun 4, 2018
@snicoll snicoll modified the milestones: 2.0.3, 2.0.x Jun 4, 2018
@nosan
Copy link
Contributor Author

nosan commented Jun 4, 2018

@snicoll done.

Do you still need ObjectProvider<ExecuteListenerProvider[]> executeListenerProviders in a separate PR ?

@snicoll
Copy link
Member

snicoll commented Jun 4, 2018

Thank you @nosan

Do you still need ObjectProvider<ExecuteListenerProvider[]> executeListenerProviders in a separate PR ?

It's a very small change, I'll polish behind if necessary

@snicoll snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Jun 4, 2018
@snicoll
Copy link
Member

snicoll commented Jun 4, 2018

Do you still need ObjectProvider<ExecuteListenerProvider[]> executeListenerProviders

There is jooqExceptionTranslatorExecuteListenerProvider bean defined in the same auto-config so the ObjectProvider wrapper is not required.

snicoll added a commit that referenced this pull request Jun 4, 2018
* pr/13331:
  Auto-configure jOOQ with TransactionListenerProvider
@snicoll snicoll modified the milestones: 2.0.x, 2.0.3 Jun 4, 2018
@snicoll snicoll closed this in 2000348 Jun 4, 2018
@nosan nosan deleted the gh-13329 branch April 23, 2019 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants