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

Prevent overwriting params by given query #2045

Merged
merged 2 commits into from Jun 4, 2016

Conversation

Projects
None yet
2 participants
@namusyaka
Member

namusyaka commented Jun 4, 2016

The behavior seems not to overwrite params by given query until 0.13.0.beta2
I think this behavior is a bug, so fixed.
/cc @padrino/core-members Thoughts?

@namusyaka namusyaka added the bug label Jun 4, 2016

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Jun 4, 2016

Member

I agree that the behavior is bad but I'm concerned about #to_sym on user-provided param keys.

Member

ujifgc commented Jun 4, 2016

I agree that the behavior is bad but I'm concerned about #to_sym on user-provided param keys.

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Jun 4, 2016

Member

Ah, yes.

Member

namusyaka commented Jun 4, 2016

Ah, yes.

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Jun 4, 2016

Member

@ujifgc How about this?

Member

namusyaka commented Jun 4, 2016

@ujifgc How about this?

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Jun 4, 2016

Member

This is unnecessary array allocation inside a loop. 😁

Member

ujifgc commented Jun 4, 2016

This is unnecessary array allocation inside a loop. 😁

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Jun 4, 2016

Member

Yep, however I think significant_variable_names may be used by users.

Member

namusyaka commented Jun 4, 2016

Yep, however I think significant_variable_names may be used by users.

@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Jun 4, 2016

Member

If I change data type of significant_variable_names on this PR, we should announce it at next releasing.

Member

namusyaka commented Jun 4, 2016

If I change data type of significant_variable_names on this PR, we should announce it at next releasing.

Don't symbolize given query key
Take care of this incompatible change
@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Jun 4, 2016

Member

@ujifgc Just force pushed.

Member

namusyaka commented Jun 4, 2016

@ujifgc Just force pushed.

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Jun 4, 2016

Member

Looks good.

Member

ujifgc commented Jun 4, 2016

Looks good.

@namusyaka namusyaka merged commit 68a65af into master Jun 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@namusyaka

This comment has been minimized.

Show comment
Hide comment
@namusyaka

namusyaka Jun 4, 2016

Member

Thanks!

Member

namusyaka commented Jun 4, 2016

Thanks!

@namusyaka namusyaka deleted the fix-params-behavior branch Jun 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment