hookdeck-cli
hookdeck-cli copied to clipboard
refactor: Config parsing & precedence
This is a fairly impactful PR as it involves a breaking change as well as a lot of code changes related to pkg/config
which impacts every command. Some key pointers:
Global & local config
Previously, we maintain 2 config scope: global & local config. Local config can be empty. This is the precedence for config values:
- accept value from flag
- read from local
- read from global
Because of this, we can also save things in the local config to override the global config. For example, $ hookdeck project use --local
will store workspace_id
value in the local config so that future commands in this scope will use the selected project. In this case, the local config file could be as short as:
workspace_id = "tm_123"
Changes
This PR removes the concept of global & local config. There's only 1 singular config. If there's a config within the local scope ($PWD/.hookdeck/config.toml
) or if the user provides the --config /path/to/config.toml
flag, then the CLI will use this config file only and ignore the global config file should one exist.
Test & refactor
Besides code to make the changes in the section above, I also refactored pkg/config
with 2 main concerns:
- Make the code more testable (for example, using
ConfigFS
interface to mock different filesystem scenarios) - Reduce public API for the package to the minimum
With that, I also added quite a bit of tests to ensure things behave correctly. Some main shoutouts:
-
TestGetConfigPath
will test that the CLI will choose the correct config file from different user input / filesystem scenarios (whether config flag is passed, whether a local config exists, etc) -
TestInitConfig
will test what the final config looks like with different config files. You can see different scenarios inpkg/config/testdata
. There's a README that gives some explanation for each scenario. Please feel free to add more if there's anything you'd like to test. -
[ ] Add more config scenarios. Something I haven't considered: backward compatibility.
-
[ ] Should we consider adding config versioning moving forward?
-
[ ] The tests currently place a very strong emphasis on profile. There's no test for other config cases like LogLevel, Base URLs, etc. Is there anything you'd like to test more? Feel free to add your own if you'd like, or comment here.