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

Add '-w workers' command line option. #1395

Merged
merged 3 commits into from
Nov 6, 2022
Merged

Conversation

abascom
Copy link
Contributor

@abascom abascom commented Nov 2, 2022

Add an optional command line parameter to be able to specify the maximum number of thread pool workers given to the thread pool executor.

Add an optional command line parameter to be able to specify the maximum
number of thread pool workers given to the thread pool executor.
@fhoeben
Copy link
Collaborator

fhoeben commented Nov 3, 2022

You indicate the minimum number of workers must be 5, why is that important? Could you add a check to the code to throw an error if a too small value is provided?

Can you also add the change to the ReleaseNotes page?

@abascom
Copy link
Contributor Author

abascom commented Nov 3, 2022

Thanks for the feedback!

You indicate the minimum number of workers must be 5, why is that important?
This is due to the ThreadPoolExecutor created in fitnesse/FitNesse.java having a corePoolSize argument of 5.

Could you add a check to the code to throw an error if a too small value is provided?
I elected not to do this as it is consistent with the behavior of the port argument. The port is just passed along to the ServerSocket which will throw an IllegalArgumentException if the port is out of range. Similarly the ThreadPoolExecutor will throw an IllegalArgumentException already if the maximumPoolSize is less than the corePoolSize.

Can you also add the change to the ReleaseNotes page?
I will do this and update the PR.

@fhoeben fhoeben merged commit b851f6e into unclebob:master Nov 6, 2022
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