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

Issue 12142 #12148

Open
wants to merge 1 commit into
base: 5.1.x
Choose a base branch
from
Open

Issue 12142 #12148

wants to merge 1 commit into from

Conversation

xpusostomos
Copy link

// right now grails also breaks it up by also adding:
// "[a[0]:[b:1], a[1]:[b:2]]"
// Personally, I think that's useless, but whether you want to retain it, I don't know.
assert params != null
Copy link
Member

Choose a reason for hiding this comment

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

asserting that params is not null may not make sense here. In order for it to be null, the new GrailsParameterMap(request) would have to return null, and then a NullPointerException would have been thrown at line 504 I think. Is that correct?

Copy link
Author

Choose a reason for hiding this comment

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

Sounds reasonable, I was just following the existing pattern.

@CLAassistant
Copy link

CLAassistant commented Nov 3, 2021

CLA assistant check
All committers have signed the CLA.

@puneetbehl puneetbehl changed the base branch from master to 5.1.x November 26, 2021 09:19
@puneetbehl
Copy link
Contributor

@xpusostomos Thank you for taking time to send this pull request. I am a little confused that whether this is about adding a test for functionality which currently does not work? or it is just an addition of new test whereas the functionality is working fine?

@xpusostomos
Copy link
Author

I raised an issue where I think grails should behave differently. I was asked to provide a pull request for a test case for how I think it should work. This I did here.

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.

4 participants