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

Support named strings as inlined map keys #83

Merged
merged 1 commit into from
Dec 21, 2024
Merged

Conversation

dsnet
Copy link
Collaborator

@dsnet dsnet commented Dec 21, 2024

It should be possible to support map[~string]T as an inlined fallback so long as the string type does not implement marshal/unmarshal methods.

It should be possible to support map[~string]T as an inlined fallback
so long as the string type does not implement marshal/unmarshal methods.
@dsnet
Copy link
Collaborator Author

dsnet commented Dec 21, 2024

@nickkhyl, my apologies for the delay, but I wanted this to go in after a lot of the recent changes for v1 compatibility.

I'm not sure how to push directly to your PR (as I think I would need write privileges to your fork). We can either:

  • have you incorporate the changes in this PR into your PR, or
  • we can submit this PR.

Either approach works for me. The commit in this PR has you listed as the author.

@dsnet
Copy link
Collaborator Author

dsnet commented Dec 21, 2024

This is a rebased version of @nickkhyl original PR in #54.

@nickkhyl
Copy link
Contributor

Thank you @dsnet for rebasing it for me. The fork is tailscale/json, and I (incorrectly) assumed that our engineers have access to it by default. I'll have that fixed, just in case someone else would like to contribute in the future.

In the meantime, I think we should submit this PR since the work has already been done here.

@dsnet dsnet merged commit 27d2cb3 into master Dec 21, 2024
8 checks passed
@dsnet dsnet deleted the inlined-map-string-kind branch December 21, 2024 23:44
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