-
Notifications
You must be signed in to change notification settings - Fork 582
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
refactor: Remove "Opts" abbreviation #92
Conversation
Codecov Report
@@ Coverage Diff @@
## main #92 +/- ##
==========================================
+ Coverage 68.84% 71.60% +2.75%
==========================================
Files 85 89 +4
Lines 3396 3578 +182
Branches 55 55
==========================================
+ Hits 2338 2562 +224
+ Misses 861 793 -68
- Partials 197 223 +26
Continue to review full report at Codecov.
|
Having a mixture of abbreviations in the codebase reduces clarity. Although opts is common for options, I'd rather set a precedent of clarifying verbosity.
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 like it, thanks for improving the readability @kylecarbs !
@@ -27,7 +27,7 @@ const ( | |||
// The initialization overrides listener handles, and detaches | |||
// the channel on open. The datachannel should not be manually | |||
// mutated after being passed to this function. | |||
func newChannel(conn *Conn, dc *webrtc.DataChannel, opts *ChannelOpts) *Channel { | |||
func newChannel(conn *Conn, dc *webrtc.DataChannel, opts *ChannelOptions) *Channel { |
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.
consider following up with a ruleguard linter rule that checks that types don't end in Opts, and suggesting Options instead
Having a mixture of abbreviations in the codebase reduces
clarity. Although opts is common for options, I'd rather
set a precedent of clarifying verbosity.