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

Update regression tests for 931130. #1388

Merged

Conversation

@studersi
Copy link
Contributor

commented May 4, 2019

See pull request #1350 for more information.

@franbuehler
Copy link
Collaborator

left a comment

Tests pass now. PR can be merged.

5 passed in 1.84 seconds

@fgsch

This comment has been minimized.

Copy link
Collaborator

commented May 4, 2019

Looks good.

Could you possible add a negative test (a test that does not match) that uses a scheme in the query string but shares the host?

@dune73 dune73 added the Needs action label May 5, 2019

@fgsch

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

Alternatively, if you are happy @dune73 we can merge this as is and add the positive test in a separate PR.

@dune73

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

The negative test, you mean @fgs? As long as said test is coming, I do not mind either way.

@studersi : When do you have time for this?

@studersi

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2019

That's fine with me. You can merge this pull request, open a new issue, and assign it to me. I can probably write the positive test this weekend.

@dune73 dune73 merged commit 5a64d9b into SpiderLabs:v3.2/dev May 7, 2019

1 check passed

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

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

Thank you for the PR and thank you for volunteering. I have now merged thi PR and opened #1404. We can not assign to non-project people unfortunately. Glad we could merge this; too many open PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.