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

feat: remove mut #28

Merged
merged 1 commit into from
Aug 25, 2023
Merged

feat: remove mut #28

merged 1 commit into from
Aug 25, 2023

Conversation

KSXGitHub
Copy link
Contributor

@anonrig Is there any reason origin must take mutable self? If not, this can be merged. If there is a reason, feel free to close this PR.

@anonrig anonrig merged commit 91713a4 into ada-url:main Aug 25, 2023
6 checks passed
@KSXGitHub KSXGitHub deleted the url-origin-self branch August 25, 2023 11:11
@KSXGitHub
Copy link
Contributor Author

@anonrig So you really just added mut there by mistake then?

@anonrig
Copy link
Member

anonrig commented Aug 25, 2023

@anonrig So you really just added mut there by mistake then?

I think it had to do with origin returning a new string owned by the C, but I don't remember.

@KSXGitHub
Copy link
Contributor Author

@anonrig But the origin method still doesn't modify the internal state at all, right?

I think it had to do with origin returning a new string owned by the C

The origin method already returns &str, which is an immutable reference. The only guarantee you need to make is that the string must live at least as long as the Url struct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anonrig anonrig anonrig approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KSXGitHub @anonrig