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.

Sign up for GitHub

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

Jump to bottom

New Modules: URI-Builder & Hamcrest Matchers #4

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

New Modules: URI-Builder & Hamcrest Matchers #4

wants to merge 8 commits into from

Conversation

ja-fra
Copy link
Member

@ja-fra ja-fra commented Apr 17, 2019

New Modules:

  • URI-Builder
  • Hamcrest Matchers

@ja-fra ja-fra requested a review from westei April 17, 2019 07:46
@ja-fra ja-fra requested a review from westei December 9, 2019 14:29
@westei
merged the master back into this branch. Had to resolve several confl…
e44416e 
…icts in pom.xml files and update versions of others. Also changed all groupIds to io.redlink.utils by directly using the root pom.xml as parent (as this scheme was adapted by other modules added in the meantime.

NOTE: I do get a failing Test `testSpecialChars` if the URI Builder as a leading `/ ` is missing in the URLs path. Will add more information to the discussions of the pull request
@westei
Copy link
Member

westei commented Dec 13, 2019

@ja-fra I merged master back into this branch. You might want to check the changes I have made to the pom files ...

... but more Important ...

there is a failing Test testSpecialChars in the URI Builder as a leading / is missing in the URLs path. A simple fix for this would just to add a leading / in the UriBuilder#joinPath(..) method, but this is only true for absolute URIs. Relative URIs might not want to have this /.

I gave this some throughs and I am not 100% sure if one can correctly reconstruct the state after splitting a parsed path. Maybe parsing the scheme to this method and adding a leading / only if the scheme != null?

@ja-fra ja-fra changed the base branch from master to main March 21, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@westei westei westei approved these changes

Assignees
No one assigned
Labels
enhancement
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ja-fra @westei