[WIP] Refactor as importable module #124
Conversation
This turns the `main` function into strictly CLI parsing
This is potentially controversial, but the idea is that skipping the extension flags is considered a deprecated feature, so moving it to the `main` function instead of the exported `Run` will allow `Run` to move forward without needing to have a backwards-compatible interface.
This way `Run()` doesn't rely on calling `log.Fatal()` and instead returns an error upstream for easier usage in modules
Functions that need to log out debug messages now take a `*log.Logger` interface and use that. This should help should you wish to call those functions in a modular way with a different logger, (`hc-log`, `logrus`, etc)
| spdx spdxFlag, | ||
| holder string, | ||
| license string, | ||
| licensef string, | ||
| year string, |
There was a problem hiding this comment.
Combining stuff like holder, year, and license into a licenseData struct and passing that in would probably feel more appropriate.
| licensef string, | ||
| year string, | ||
| verbose bool, | ||
| checkonly bool, |
There was a problem hiding this comment.
checkonly gets a bit weird here, since this function currently calls os.Exit(1) if there are files that need changes, which is definitely not consumable. Perhaps an alternative would be to return a boolean that represents if changes are needed or were made. From there, a consumer of this function could infer that if they pass checkonly = true and get back a true, it means files need to be modified. If checkonly = false and they get back a true, it means files were modified.
I don't love it, but I do like it more than returning an error. Just thinking out loud, though 🙂
|
The bigger issue with making addlicense usable as an importable module is actually getting the importable pieces out of the main package. That's going to be a much bigger lift than just moving things out of the main func. It's sort of restructuring the whole repo. And honestly more than I've probably got bandwidth to review as well right now :-\ |
I totally empathize with that! Trying to maintain open source projects in your free time with competing priorities is something I know all to well 🙂 I may tentatively plan on forking the code base as it currently sits, since we'd like to use it for some internal tooling. That said, I may still put some PRs up here and there for smaller things that would be applicable to the community (e.g., support for additional file types). |
|
Sounds good. And if I do get some free cycles, I may try doing some of the refactor in a separate branch just to see what it'd be like. |
This is largely just to test out some of the discussion points going on in #123. It is not merge-ready and should be thoroughly vetted before any API is exposed.