-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add IPAddr type #427
Add IPAddr type #427
Conversation
Codecov Report
@@ Coverage Diff @@
## master #427 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 127 128 +1
Lines 2188 2192 +4
=====================================
+ Hits 2188 2192 +4
Continue to review full report at Codecov.
|
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 how you covered the boundaries, good job looks good, easy to read. Something super minor, i ended up not using Fprintln and just use Fprint so i dont have to add and expect \n everytime :)
@starfallz ah nice call! Will do this in my other labs as well. Just a reminder to update the labels (under review / changes requested / approved by colleague) accordingly when you are reviewing! |
be69596
to
11912e0
Compare
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.
Looking really good. A couple of super minor things and then this should be ready for merging. Just ping me once this is done and I will try to take a look asap 👍
11912e0
to
ffd5248
Compare
Implements fmt.Stringer Closes Lab 05 (anz-bank#420)
ffd5248
to
382d7ef
Compare
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
Implements fmt.Stringer
Fixes #420
Review of colleague's PR #534