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

Update BlockHound version to be compatible with JDK 17 #3771

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

qwwdfsad
Copy link
Collaborator

@qwwdfsad qwwdfsad commented Jun 2, 2023

  • Also, exclude integration from docs, it shouldn't be there

Fixes #3701

* Also, exclude integration from docs, it shouldn't be there

Fixes #3701
@qwwdfsad qwwdfsad requested a review from dkhalanskyjb June 2, 2023 15:13
@qwwdfsad
Copy link
Collaborator Author

qwwdfsad commented Jun 2, 2023

Note: it doesn't work with JDK 20+ though

@@ -10,6 +10,9 @@ import kotlinx.coroutines.scheduling.*
import reactor.blockhound.*
import reactor.blockhound.integration.*

/**
* @suppress
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seeing this, I began to wonder whether we should document the class instead since it is possible to sidestep the ServiceLoader mechanism with BlockHound and apply the integrations manually, but it looks like nobody actually uses the integration by name (https://github.com/search?q=CoroutinesBlockHoundIntegration&type=code), and it's not expected that the integrations will be documented (https://github.com/reactor/BlockHound/tree/master/agent/src/main/java/reactor/blockhound/integration), so ok.

@@ -10,6 +10,9 @@ import kotlinx.coroutines.scheduling.*
import reactor.blockhound.*
import reactor.blockhound.integration.*

/**
* @suppress
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seeing this, I began to wonder whether we should document the class instead since it is possible to sidestep the ServiceLoader mechanism with BlockHound and apply the integrations manually, but it looks like nobody actually uses the integration by name (https://github.com/search?q=CoroutinesBlockHoundIntegration&type=code), and it's not expected that the integrations will be documented (https://github.com/reactor/BlockHound/tree/master/agent/src/main/java/reactor/blockhound/integration), so ok.

@qwwdfsad qwwdfsad merged commit 0288b72 into develop Jun 5, 2023
@qwwdfsad qwwdfsad deleted the upd-blockhound branch June 5, 2023 10:49
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