-
Notifications
You must be signed in to change notification settings - Fork 74
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
Idiomatic #46
base: master
Are you sure you want to change the base?
Idiomatic #46
Conversation
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.
moving the code from gosnmp.go to snmp.go makes it hard for me to review the changes -- can you split that diff into two? One for the changes in the code, and then one more to rename?
Also, logging in general is a good thing -- not sure what's the best way to log stuff in Go these days but I am not a huge fun of dropping it altogether.
otherwise excellent job in cleaning up the code :)
decode.go
Outdated
} | ||
|
||
//// Parses UINT16 |
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.
If this is no longer used, then remove the code
helper.go
Outdated
ret.Bytes = bytes[1:] | ||
return | ||
} | ||
//// parseBitString parses an ASN.1 bit string from the given byte slice and returns it. |
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.
Same as before, if this is no longer used, drop the code
Thanks! How can I split the diff as you mentioned? |
…ing the UDP connection. Also added new example_test.go file to add examples to GoDoc documentation.
Made breaking API changes that need to be merged.
Changed API to be more idiomatic for Gophers.