Refactoring: Simplify adding new configuration properties #2107
Conversation
This eliminates argument order mistakes, and also lets us add new inputs without having to update every test.
This means that we don't need to update irrelevant test cases when we add a new configuration property.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| logger: Logger; | ||
| } | ||
|
|
||
| type GetDefaultConfigInputs = Omit< |
There was a problem hiding this comment.
TIL about the Omit type 🤯
There was a problem hiding this comment.
We could use Omit/Pick to refactor the StatusReportBase and all those other interfaces later, perhaps...
There was a problem hiding this comment.
There are quite a few of these nice types :) https://www.typescriptlang.org/docs/handbook/utility-types.html
| "configFile" | "configInput" | ||
| >; | ||
|
|
||
| type LoadConfigInputs = Omit<InitConfigInputs, "configInput"> & { |
There was a problem hiding this comment.
What does this & operator do here? Thought it represented bitwise AND but I can't really make sense of it in this context.
There was a problem hiding this comment.
These are intersections. Though I wonder if this should be a union type (using |).
There was a problem hiding this comment.
I believe we want intersections, i.e. we want the type to include the properties from both the left hand side and the right hand side. For instance if we have (1) { a: 1 } and (2) { a: 1, b: "abc" }, then (1) and (2) would satisfy { a: int } | { b: string } but only (2) would satisfy { a: int } & { b: string }.
This PR refactors the way we initialize and test configurations to make it easier to add new configuration properties. It shouldn't have any user visible effects. Specifically, adding a new configuration property should not require updating irrelevant tests.
Merge / deployment checklist