Skip to content

Support for optional query params #27

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mrzepinski
Copy link

The query params could be optional
I've added an option to test method to support that.

optionalQueryParams is ready to use

@mrzepinski
Copy link
Author

@troch Is there a way to merge this and release new version?

@troch
Copy link
Owner

troch commented Aug 1, 2018

Hi @mrzepinski,

Sorry for being so late to come back to you. Your change is a good addition, however I don't think the option name optionalQueryParams is the best: query params are optional in the sense that not all listed query params need to be provided. Given that another option is called strictTrailingSlash, what about strictQueryParams?

I also wouldn't mind for it to be false by default (and I'll release a new major version).

Does it sound OK to you?

@mrzepinski
Copy link
Author

@troch I also have problems with the optionalQueryParams, so strictQueryParams would be very good option. Do you want to make the change?

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