Skip to content

refactor: clean up and consolidate config values in codebase #29

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

Merged
merged 34 commits into from
Mar 28, 2024

Conversation

Parkreiner
Copy link
Member

@Parkreiner Parkreiner commented Mar 13, 2024

Not quite the CoderAppConfig.templateName update, but I felt like the codebase was on the cusp of getting too complex for its own good. I think that some of the complexity is a bit unavoidable, since we still have to deal with Backstage APIs, but this PR consolidates a lot of it into fewer files

Changes

  • Drops the total number of config types we've got in the codebase, shrinking them down to two main ones: CoderAppConfig and CoderWorkspacesConfig
    • CoderAppConfig isn't really changed, but the new CoderWorkspacesConfig is the old CoderEntityConfig and CoderWorkspacesConfig combined together
    • Also decoupled a lot of the type definitions, because while they're related, having them be defined so tightly felt like it could make the code more fragile (especially with the templateName change on the horizon)
  • Renamed useCoderEntityConfig to useCoderWorkspacesConfig and useCoderWorkspaces to useCoderWorkspacesQuery
  • Added a CoderWorkspacesConfig.creationUrl property which will always be defined
    • This was the old serialization logic, but rather than have you import in every single time, it felt best to move it right next to the source
  • Moved the dynamic config logic to useCoderWorkspacesConfig
  • Updated tests and mock data to account for all the changes

Notes

  • Once this PR is done, updating the logic to account for an optional templateName should be quick. Hoping to get that in before Wednesday.
  • Once the main code is approved, I'll make some quick commits for the README to account for the API changes, and then squash in
  • One of Kira's concerns was that these changes could have ramifications for the untested coder workspaces card components.
    • I'm opening this up for review now, just to limit the amount of code people will need to look at. But even after this gets approved, I won't merge it in until I have a better test suite for the card components (to be covered in another PR)

@Parkreiner Parkreiner self-assigned this Mar 13, 2024
@Parkreiner Parkreiner changed the title chore: refactor and redefine config values for coder plugin refactor: clean up and consolidate config values in codebase Mar 13, 2024
@Parkreiner Parkreiner marked this pull request as ready for review March 13, 2024 23:09
@Parkreiner Parkreiner marked this pull request as draft March 14, 2024 13:11
@Parkreiner Parkreiner marked this pull request as ready for review March 23, 2024 18:28
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

@Parkreiner Parkreiner merged commit b19c08e into main Mar 28, 2024
@Parkreiner Parkreiner deleted the mes/template-name-update branch March 28, 2024 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants