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

Document that uri throws. #353

Closed

Conversation

htmldoug
Copy link
Contributor

@htmldoug htmldoug commented Jun 10, 2019

Pull Request Checklist

  • Have you read through the contributor guidelines?
  • Have you signed the Typesafe CLA?
  • Have you squashed your commits?
  • Have you added copyright headers to new files?
  • Have you checked that both Scala and Java APIs are updated?
  • Have you updated the documentation for both Scala and Java sections?
  • Have you added tests for any changed functionality?

Fixes

Documents the surprising behavior of #267.

Purpose

Documents that lazy val uri: URI throws.

Background Context

@marcospereira suggested in #267 (comment) that def url(url: String): StandaloneWSRequest should be tolerant of urls which include query params. I submit #288, but was not successful in getting it reviewed a second time and merged. URLs are hairy and I can understand why you may not trust the robustness of a community provided solution. #288 has been unmerged for about a year now, and marked status: backlog so I've closed it.

Instead, this PR just documents the current behavior, hopefully to prevent others from discovering it in prod as I did. I've also added a failing unit test.

References

@htmldoug htmldoug mentioned this pull request Jun 10, 2019
7 tasks
@htmldoug htmldoug force-pushed the issue-267-document-uri-throws branch from 10ff0d3 to 794a336 Compare June 27, 2019 23:17
@htmldoug
Copy link
Contributor Author

htmldoug commented Feb 4, 2020

Closing due to lack of review.

@htmldoug htmldoug closed this Feb 4, 2020
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