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

minor code tyding up and using new APIs #25

Merged
merged 3 commits into from
Apr 12, 2024
Merged

minor code tyding up and using new APIs #25

merged 3 commits into from
Apr 12, 2024

Conversation

mvdan
Copy link
Collaborator

@mvdan mvdan commented Apr 2, 2024

(see commit messages - please do not squash)

Note that the last commit, using OnceFunc, is up for debate - it simplifies the code a bit and gives us perhaps better panic semantics, but it is slightly slower too, although not always for some reason.

@mvdan mvdan requested a review from dsnet April 2, 2024 06:32
@dsnet
Copy link
Collaborator

dsnet commented Apr 11, 2024

The performance hit of sync.OnceFunc is unfortunate. Given that it doesn't cleanup the code much, I'm inclined to keep the older code.

All other changes LGTM.

Before submitting, try keeping the commit titles consistent with historical titles (i.e., we don't prefix with the component name). It's unfortunate that I didn't start this project off with the typical Go project convention, but that ship sailed and consistency is nice.

errIgnoredField was unused, two src values were unused,
and the switch in checkDelim can work directly on the value.
The experiment is now fully public.
@mvdan
Copy link
Collaborator Author

mvdan commented Apr 12, 2024

Thanks, done.

@mvdan mvdan merged commit 8868a69 into master Apr 12, 2024
8 checks passed
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