Implement lab05 stringer for anz-bank go-course #526
Conversation
undrewb
requested review from
anzboi,
anzdaddy,
camscale,
joshcarp,
juliaogris,
linuxmonk,
pentaphobe and
rokane
as code owners
Update test to fix newline issue
Codecov Report
@@ Coverage Diff @@
## master #526 +/- ##
=======================================
Coverage 100% 100%
=======================================
Files 73 187 +114
Lines 1273 3520 +2247
=======================================
+ Hits 1273 3520 +2247
Continue to review full report at Codecov.
|
|
LGTM As a side note - you can get rid of the |
|
I advise against changing user@machine:~$ lab05-fprintln
127.0.0.1
user@machine:~$ _user@machine:~$ lab05-println
127.0.0.1user@machine:~$ _ |
| main() | ||
|
|
||
| want := strconv.Quote("127.0.0.1\n") | ||
| got := strconv.Quote(buf.String()) |
There was a problem hiding this comment.
Actually, this seems to be a common pattern among go-course PRs. Is it recommended somewhere to do it this way?
There was a problem hiding this comment.
It saves you typing `""127.0.0.1\n"" - which is the output of main, which needs to be tested for coverage. that's all.
Change assert parameter order to be expected then actual Fix formatting of test case structs
05_stringer/undrewb/main_test.go
Outdated
| input: IPAddr{68, 2, 44, 125}, | ||
| want: "68.2.44.125", | ||
| }, | ||
| {name: "three octet addr", |
There was a problem hiding this comment.
The indentation on this is inconsistent with the rest, can you please fix this so this line starts on the one below
05_stringer/undrewb/main_test.go
Outdated
| want: "68.2.44.125", | ||
| }, | ||
| { | ||
| name: "three octet addr", |
There was a problem hiding this comment.
File is not goimports-ed (from goimports)
| name: "three octet addr", | |
| name: "three octet addr", |
|
The go-course is now closed. Thank you very much for participating. |
Fixes #525
Review of colleague's PR #511
Changes proposed in this PR: