Skip to content

Conversation

@1majom
Copy link
Contributor

@1majom 1majom commented Aug 12, 2024

I know this is not an important change, but I came across it while changing the api for my own project and it might help others who want to do the same.

The url's join method overwrites the previous join. So e.g. url.join("origin/").join("vaj") won't make an url like kiscica.hu/origin/vaj, but kiscica.hu/vaj.

@kixelated
Copy link
Collaborator

TIL URL::join is weird. But shouldn't the existing code work because of the trailing slash? That's probably why I added it in the first place.

@1majom
Copy link
Contributor Author

1majom commented Aug 16, 2024

The existing code will work because this is consistent, so for all paths it will be only moq.relay/ not moq.relay/origin/. I just wanted to correct this because when I made more complicated paths I saw that double joins won't get us the wanted outcome.

But if you prefer it that way, instead of sticking to the origin/ part, the affected lines could be replaced simply with "self.url.join(namespace)?". That way it will do the same as now without the double joins.

@kixelated
Copy link
Collaborator

I just wasn't sure if anything was broken. Thanks for the improvement!

@kixelated kixelated merged commit e6a92a8 into moq-dev:main Aug 16, 2024
This was referenced Aug 16, 2024
@github-actions github-actions bot mentioned this pull request Sep 3, 2024
@github-actions github-actions bot mentioned this pull request Oct 1, 2024
@1majom 1majom deleted the api_joins branch October 29, 2024 14:15
ajay-s-2502 pushed a commit to ajay-s-2502/moq-rs that referenced this pull request Nov 22, 2024
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