-
Notifications
You must be signed in to change notification settings - Fork 0
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
Recommend the stdlib atomic types instead of go.uber.org/atomic #3
Conversation
f486e62
to
06bae45
Compare
src/atomic.md
Outdated
[sync/atomic]: https://pkg.go.dev/sync/atomic | ||
<!-- | ||
|
||
TODO: It'd be nice to have a shared library that provided a typed atomic value instead of having |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's https://pkg.go.dev/sync/atomic#Pointer for that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah I see that file is proposing a typed value, not a pointer. That being said, could we actually leave out discussion like this from within the files? Might be better placed elsewhere like an issue or ticket, just so these don't add up and become harder to action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seconded on issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this.
style.md
Outdated
|
||
<!-- | ||
|
||
TODO: It'd be nice to have a shared library that provided a typed atomic value instead of having |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that style.md
is just a generated cat
of all the individual files in src
. I will update .gitattributes
so it shows up as a generated file in the PR Files Changed view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@pjjw Ping ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh saw eric's LGTM and agree! let me just register that intent properly
Uber's package predates the new types added in Go 1.19. The new stdlib types provide more or less the same functionality as Uber's.