Skip to content

Add processor based calculation for the eventing threadpool#38

Open
royteeuwen wants to merge 4 commits intomasterfrom
SLING-12349
Open

Add processor based calculation for the eventing threadpool#38
royteeuwen wants to merge 4 commits intomasterfrom
SLING-12349

Conversation

@royteeuwen
Copy link

No description provided.

@royteeuwen
Copy link
Author

@kwin my PR didn't build because of JDK 21, but only upgrading Mockito to the latest version didn't fix that (#36).

Can you verify if this PR is ok and your pr can be closed?

@royteeuwen
Copy link
Author

The build still seems to fail, on Java 11 linux, while the same issue does not occur for me on Java 11... Will have to investigate further

Copy link
Contributor

@joerghoh joerghoh left a comment

Choose a reason for hiding this comment

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

in general looks good, thanks Roy!

I am worried a bit about the change in the configuration from int to a double, can you check that it still works when a INT is provided?

Doing a functional change next to the dependency updates make this change hard to assess. Can you split the PR into the functional change and another one for the updates to the pom?

@kwin
Copy link
Member

kwin commented Oct 25, 2024

@royteeuwen Do you have some time to pick up the comments from @joerghoh ? Would be great to merge this in.

@royteeuwen
Copy link
Author

@royteeuwen Do you have some time to pick up the comments from @joerghoh ? Would be great to merge this in.

Resolved and answered his comments.

I also removed the update of the parent of this PR and just made the PR build on JDK11 only. Still it seems that the windows 11 build fails, only the linux one succeeds so far. (don't think its related to my PR though). Any suggestions on going forward?

@kwin
Copy link
Member

kwin commented Oct 28, 2024

We probably need to get the ITs stable first, I made an attempt in #37.

@sonarqubecloud
Copy link

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.

3 participants