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

chore: move tools to src/tools #1729

Closed
wants to merge 3 commits into from
Closed

Conversation

daniellacosse
Copy link
Contributor

No description provided.

@daniellacosse daniellacosse marked this pull request as ready for review September 27, 2023 15:18
@daniellacosse daniellacosse requested review from a team as code owners September 27, 2023 15:18
@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

All modified lines are covered by tests ✅

📢 Thoughts on this report? Let us know!.

@fortuna fortuna requested a review from jyyi1 September 28, 2023 14:36
Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

It helpful to check if the Electron apps still work well, and that we can still build their native components.
Perhaps @jyyi1 can help.

@fortuna
Copy link
Collaborator

fortuna commented Sep 28, 2023

This is good, incremental progress. I see two simple follow-ups to this PR

  • rename src/ to something like client_app/
  • move src/infrastructure/ to the root

@fortuna
Copy link
Collaborator

fortuna commented Oct 5, 2023

Is this PR blocked on anything?

@daniellacosse
Copy link
Contributor Author

Is this PR blocked on anything?

I think you wanted @jyyi1 to verify?

@fortuna
Copy link
Collaborator

fortuna commented Oct 5, 2023

It doesn't have to be @jyyi1 . If you can validate at least that we can build the native components for all platforms, it should be fine. QA can catch if there was an issue with the functionality, but ideally we would check that too.

@daniellacosse
Copy link
Contributor Author

daniellacosse commented Oct 5, 2023

It doesn't have to be @jyyi1 . If you can validate at least that we can build the native components for all platforms, it should be fine. QA can catch if there was an issue with the functionality, but ideally we would check that too.

I don't really see the point to be honest - we had to revert the resources change because it broke the build. I think maybe the monorepo is too ambitious. I'm going to stick with small, incremental improvements from here on out. Impossible to get team buy-in otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants