Skip to content
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

Decouple discobot from commonwealth #6203

Merged
merged 9 commits into from
Jan 5, 2024

Conversation

Rotorsoft
Copy link
Contributor

Removes all references to commonwealth service

Link to Issue

Closes: #6196

Description of Changes

  • Removes imported types from commonwealth service
  • Cleans project structure and tsconfig - one step to standards

"How We Fixed It"

Copy link
Collaborator

@timolegros timolegros left a comment

Choose a reason for hiding this comment

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

While cleaning up the tsconfig and restructuring is good I don't think removing the model attributes import is worth it here. We've had bugs introduced because these queries weren't properly typed.

Seems like this change should go in when models are moved to /libs. I think this goes for any package that uses commonwealth models. We shouldn't have to loosen types to decouple packages.

@Rotorsoft
Copy link
Contributor Author

Rotorsoft commented Jan 4, 2024

While cleaning up the tsconfig and restructuring is good I don't think removing the model attributes import is worth it here. We've had bugs introduced because these queries weren't properly typed.

Seems like this change should go in when models are moved to /libs. I think this goes for any package that uses commonwealth models. We shouldn't have to loosen types to decouple packages.

I agree, this is part of the tradeoffs when breaking this into small pieces, there are some chicken-egg scenarios we'll have to resolve temporarily.

This will be resolved here: #6206

@Rotorsoft Rotorsoft requested a review from timolegros January 4, 2024 15:54
@Rotorsoft Rotorsoft temporarily deployed to discobot-listener-staging January 4, 2024 15:59 Inactive
@Rotorsoft Rotorsoft requested a review from rbennettcw January 4, 2024 16:10
@Rotorsoft
Copy link
Contributor Author

@timolegros can you approve this one to merge it?

@Rotorsoft Rotorsoft merged commit d273079 into master Jan 5, 2024
7 checks passed
@Rotorsoft Rotorsoft deleted the rotorsoft/6196.decouple-discobot branch January 5, 2024 02:10
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.

Decouple discord-bot from commonwealth
3 participants