-
Notifications
You must be signed in to change notification settings - Fork 308
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
Added section on closing the context #648
Conversation
=== Closing the Context | ||
If the application requires the `ApplicationContext` to be closed at the | ||
completion of a task (all `*Runner#run` methods have been called and the task | ||
repository has been updated), set the property `spring.cloud.task.closecontext_enabled` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sure there must have been a discussion around naming this property. But, at first look, this property name especially the underscore in closecontext_enabled
is confusing. Could you help me understand the reasoning of this naming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change it to closeContextEnabled. In this case I was following the previous usage in the documentation.
Would that be helpful?
@@ -52,7 +52,7 @@ updated in the repository with the results. | |||
|
|||
NOTE: If the application requires the `ApplicationContext` to be closed at the | |||
completion of a task (all `*Runner#run` methods have been called and the task | |||
repository has been updated), set the property `spring.cloud.task.closecontext_enabled` | |||
repository has been updated), set the property `spring.cloud.task.closecontextEnabled` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name sounds ok to me. But, where is this implemented? Shouldn't that change there as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case the property already has the following format closecontextEnabled
. i.e. https://github.com/spring-cloud/spring-cloud-task/blob/master/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/TaskProperties.java#L68
I guess we can either use camel case or kabob case.
Rebased and merged as f7c2501 |
resolves #646