Sorry for the delay - finally got a couple hours to hop in, so this is a first start. Each commit represents a reasonably atomic change with some breadcrumbs in the commit message, however realistically the changes are often multiple commits - one commit setting things up, then the next commit applying the refactor.
Overall, again I want to stress that its great to see an actual working package, none of these things are needed but also aren't too much overhead if you know about them. It took me many years to stumble across many so hopefully this can share some of that out in an easy-to-demonstrate way.
2) refactor one command to a pattern that doesn't rely on init functions and reduces the global variable usage by eliminating a flag. This pattern allows easier integration/e2e style tests that would be able to test the cli as the user would use it without needing to resort to compiling a binary and testing that.
It'd likely be overly annoying to completely get rid of init functions, but at least if you push the only initialization to touch the root command and make all subcommands as easy as possible to interact with in isolation, the better
3) take advantage (somewhat) of go's types to create a concrete Period type that is initialized through a constructor that can test whether the given period is valid. note: this is a naive implementation as a quick-and-dirty regex, better yet would be to actually parse the dates to makes ure they're valid. Currently something like 2022-99-99 would slip through just fine
4) using a explicit _test package pattern to only test the public api. This one is still the hardest for me to be drawn into. Its always so tempting to write unit tests for internal helpers, since the tests are often easy to write. However its good practice to only test public api to leave the internals easy to refactor
JohnCoene
commented
2 years ago
Sorry for the delay - finally got a couple hours to hop in, so this is a first start. Each commit represents a reasonably atomic change with some breadcrumbs in the commit message, however realistically the changes are often multiple commits - one commit setting things up, then the next commit applying the refactor.
Overall, again I want to stress that its great to see an actual working package, none of these things are needed but also aren't too much overhead if you know about them. It took me many years to stumble across many so hopefully this can share some of that out in an easy-to-demonstrate way.
So lets talk about whats going down:
1) first was to move all the logic into internal. Anything that we wouldn't necessarily want someone else importing should pop under internal - aka pretty much everything unless you're building librar(ies) explicitly for other repos to use. https://dave.cheney.net/2019/10/06/use-internal-packages-to-reduce-your-public-api-surface and https://github.com/golang-standards/project-layout#internal
2) refactor one command to a pattern that doesn't rely on init functions and reduces the global variable usage by eliminating a flag. This pattern allows easier integration/e2e style tests that would be able to test the cli as the user would use it without needing to resort to compiling a binary and testing that.
https://stackoverflow.com/questions/56039154/is-it-really-bad-to-use-init-in-go
It'd likely be overly annoying to completely get rid of init functions, but at least if you push the only initialization to touch the root command and make all subcommands as easy as possible to interact with in isolation, the better
3) take advantage (somewhat) of go's types to create a concrete Period type that is initialized through a constructor that can test whether the given period is valid. note: this is a naive implementation as a quick-and-dirty regex, better yet would be to actually parse the dates to makes ure they're valid. Currently something like 2022-99-99 would slip through just fine
4) using a explicit _test package pattern to only test the public api. This one is still the hardest for me to be drawn into. Its always so tempting to write unit tests for internal helpers, since the tests are often easy to write. However its good practice to only test public api to leave the internals easy to refactor
https://youtu.be/UKe5sX1dZ0k?t=1206 https://dave.cheney.net/paste/absolute-unit-test-london-gophers.pdf https://testing.googleblog.com/2015/01/testing-on-toilet-prefer-testing-public.html
5) using table driven tests https://dave.cheney.net/2019/05/07/prefer-table-driven-tests as a testing pattern - so easy to add new tests!
Still TODO: