-
Notifications
You must be signed in to change notification settings - Fork 881
chore: switch to generated types #1394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
That way the renderer only takes `string` for example when rendering the name field instead of `string | number` when the interface has some fields that are strings and some fields are numbers. This will be necessary when switching to generated types since some of the fields are numbers (like the owner count on a template).
fbf0d4c
to
b98aeb6
Compare
In some places the organization ID is part of the URL but not part of the request so I separated out the ID into a separate argument in the relevant API functions. Otherwise this was a straightforward replacement where I mostly only needed to change some of the interface names (User instead of UserResponse for example) and add a few missing but required properties. Arguably `parameter_values` could be made optional but I am just passing in an empty array for now mostly just to make it obvious that we could pass stuff there if we need to. I kind of winged the template form; I am not sure what the difference between a template and template version is or why the latter comes before the former so the form just returns all the data required to create both.
Codecov Report
@@ Coverage Diff @@
## main #1394 +/- ##
==========================================
+ Coverage 67.01% 67.03% +0.01%
==========================================
Files 287 288 +1
Lines 18850 19083 +233
Branches 241 241
==========================================
+ Hits 12633 12792 +159
- Misses 4931 4985 +54
- Partials 1286 1306 +20
Continue to review full report at Codecov.
|
Except for UserAgent which seems to be purely frontend and ReconnectingPTYRequest which is not in codersdk so I am just leaving it for now.
This was implemented in 2d3dc43.
b98aeb6
to
1de1986
Compare
This reverts commit f5d1b62. This will be added in a separate PR.
I don't have time to properly review right now but I wanted to mention that I think more fields on |
Looks like you are right. Fortunately it is pretty easy, we just need to add I am not sure what the best way to tackle this is (not sure if I have to read the code to find what is optional or if everything without a |
nice! seems like |
All right the request interfaces are looking good. @Kira-Pilot I see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still learning but this looks good to me!
@code-asher yay thanks! The empty string for schedule makes sense to me. |
@Kira-Pilot we run it manually. On save is a bit excessive since it does take some time. But CI will have a failing step if you changed something and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super appreciate this!
* Make column renderer use the same type as its key That way the renderer only takes `string` for example when rendering the name field instead of `string | number` when the interface has some fields that are strings and some fields are numbers. This will be necessary when switching to generated types since some of the fields are numbers (like the owner count on a template). * Switch fully to generated types In some places the organization ID is part of the URL but not part of the request so I separated out the ID into a separate argument in the relevant API functions. Otherwise this was a straightforward replacement where I mostly only needed to change some of the interface names (User instead of UserResponse for example) and add a few missing but required properties. I kind of winged the template form; I am not sure what the difference between a template and template version is or why the latter comes before the former so the form just returns all the data required to create both. * Delete handwritten types Except for UserAgent which seems to be purely frontend and ReconnectingPTYRequest which is not in codersdk so I am just leaving it for now. * Remove implemented omitempty as a future idea This was implemented in 2d3dc43. * Add missing optionalities to generated request interfaces
Closes #1210. More information can be found in the commits!