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

W-17475148: Simple type params handler not handling certain expression param values #14113

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

elrodro83
Copy link
Contributor

  • Addendum

@elrodro83 elrodro83 requested a review from a team as a code owner January 2, 2025 14:16
@elrodro83 elrodro83 enabled auto-merge (squash) January 2, 2025 17:15
Comment on lines 46 to 49
protected boolean isExpressionValue(CreateParamBeanDefinitionRequest request) {
return request.getParam().getValue().isLeft();
return request.getParam().getValue().isLeft()
|| isContent(request.getParam().getModel());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There is something that I don't understand here. The method name is isExpressionValue and we are changing it so that static values are also considered as expressions if the parameter is a content parameter?
Maybe I'm getting it wrong, but from the test it looks like it.

Are we sure this is the right place to add the "if"? or at least maybe we need a good comment here.

Copy link

@elrodro83 elrodro83 merged commit 80516ed into master Jan 2, 2025
8 checks passed
@elrodro83 elrodro83 deleted the fix/W-17475148-b branch January 2, 2025 19:51
elrodro83 added a commit that referenced this pull request Jan 2, 2025
elrodro83 added a commit that referenced this pull request Jan 2, 2025
elrodro83 added a commit that referenced this pull request Jan 2, 2025
elrodro83 added a commit that referenced this pull request Jan 2, 2025
elrodro83 added a commit that referenced this pull request Jan 3, 2025
elrodro83 added a commit that referenced this pull request Jan 3, 2025
elrodro83 added a commit that referenced this pull request Jan 3, 2025
elrodro83 added a commit that referenced this pull request Jan 3, 2025
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