feat(config): implement global config to wire up config command#1789
feat(config): implement global config to wire up config command#1789Hweinstock wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #1789 +/- ##
============================================
+ Coverage 93.61% 93.65% +0.03%
============================================
Files 117 123 +6
Lines 6708 6886 +178
============================================
+ Hits 6280 6449 +169
- Misses 428 437 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
db54dba to
b39f26a
Compare
|
typecheck fixed in #1756 (comment) |
| }); | ||
| } | ||
|
|
||
| function buildSchemas(schema: z.ZodObject, prefix: string = ""): Map<string, z.ZodType> { |
There was a problem hiding this comment.
we could probably combine this with buildKeys above into a single traversal, but found it more readable this way.
| expect(run([])).rejects.toThrow("unable to read"); | ||
| }); | ||
|
|
||
| test("throws clear error on invalid json", async () => { |
There was a problem hiding this comment.
this test will be more useful once we have error types to signify permission issue vs parsing issue. checking the message feels flaky, so deferring this until we have error types.
| * @param schema - The zod schema to inspect. | ||
| * @returns An inspection describing the schema's structure. | ||
| */ | ||
| export function inspectZodSchema(schema: z.ZodType): ZodSchemaInspection { |
There was a problem hiding this comment.
this is pretty close to a generalization of basetype helper above now that we extract inner type. I definitely think we can unify them, but left it OOS for this PR.
| // print entire config when key is missing. | ||
| if (!args.key) { | ||
| const fullConfig = await globalConfigAccessor.toObject(); | ||
| if (fullConfig !== undefined) jsonRenderer.renderJson(fullConfig); |
There was a problem hiding this comment.
this mirrors the behavior with git config AFAICT, if the value is unset, we don't print anything.
b39f26a to
4b819e1
Compare
d60cf0d to
7fdb445
Compare
|
rebased onto create command work. |
AlexanderRichey
left a comment
There was a problem hiding this comment.
I feel that the level of complexity this PR introduces is out of proportion with the functionality gained. Let's sync on this and see if we can develop a simpler approach.
|
I have some prior art here that might be useful to look at: https://github.com/AlexanderRichey/yagss/blob/main/internal/builder/config.go |
|
converted to draft while I rework based on offline discussion from above, will take out of draft when ready for another review. |
7fdb445 to
03b6021
Compare
451366a to
f5095a3
Compare
f5095a3 to
c39237b
Compare
Problem
The CLI is missing a way to view and control global configuration. There is an existing
configcommand, but its not wired up.Solution
globalConfig/module that defines an accessor for global config. The global config is decoupled from the source of the data via an injected dependency.src/router/schemas.tsxto top level/parsingdirectory so that logic can be re-used.telemetry,telemetry.enabled, etc. All of which are derived automatically from the schema definition.Testing
Ex.
Future Work