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 missing 'havingValue="true"' for '*.enabled' to keep consistency #43641

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Dec 31, 2024

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 31, 2024
@philwebb philwebb changed the title Add missing havingValue="true" for *.enabled Add missing 'havingValue="true"' for '*.enabled' Dec 31, 2024
@philwebb
Copy link
Member

This shouldn't be necessary, at least if the javadoc is to be believed:

The string representation of the expected value for the properties. If not specified, the property must not be equal to {@code false}.

@quaff Have you found this isn't the case?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Dec 31, 2024
@quaff
Copy link
Contributor Author

quaff commented Jan 2, 2025

This shouldn't be necessary, at least if the javadoc is to be believed:

The string representation of the expected value for the properties. If not specified, the property must not be equal to {@code false}.

@quaff Have you found this isn't the case?

I guess many people would not read this javadoc, they would change true to false instead of removing it by intuition.
What if people really want some property to match explicit false?
However, I think it should keep consistency since other *.enabled conditions are restricted to havingValue="true".

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 2, 2025
@quaff quaff changed the title Add missing 'havingValue="true"' for '*.enabled' Add missing havingValue="true" for *.enabled to keep consistency Jan 2, 2025
@wilkinsona
Copy link
Member

This has the potential to be a breaking change. As such, I'm not sure it's worth making. If it is, it would have to go in 3.5 at the earliest. If we do decide to make a change like this we may want to consider some options for avoiding the re-introduction of the problem.

@wilkinsona wilkinsona added for: team-meeting An issue we'd like to discuss as a team to make progress and removed status: feedback-provided Feedback has been provided labels Jan 6, 2025
@philwebb philwebb changed the title Add missing havingValue="true" for *.enabled to keep consistency Add missing 'havingValue="true"' for '*.enabled' to keep consistency Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-meeting An issue we'd like to discuss as a team to make progress status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants