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.

Sign up for GitHub

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

Jump to bottom

refactor(infrastructure): move infrastructure code to its own package #1985

Merged
merged 31 commits into from
Apr 23, 2024
Merged

refactor(infrastructure): move infrastructure code to its own package #1985

merged 31 commits into from
Apr 23, 2024

Conversation

sbruens
Copy link
Contributor

@sbruens sbruens commented Apr 19, 2024

No description provided.

fortuna
fortuna requested changes Apr 19, 2024
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
client/package.json Outdated Show resolved Hide resolved
infrastructure/build.action.sh Outdated Show resolved Hide resolved
infrastructure/crypto.ts Show resolved Hide resolved
infrastructure/index.ts Outdated Show resolved Hide resolved
infrastructure/package.json Outdated Show resolved Hide resolved
infrastructure/test.action.sh Outdated Show resolved Hide resolved
infrastructure/test.webpack.js Outdated Show resolved Hide resolved
infrastructure/tsconfig.json Outdated Show resolved Hide resolved
@github-actions github-actions bot added size/L and removed size/XXL labels Apr 22, 2024
@sbruens sbruens marked this pull request as ready for review April 22, 2024 19:12
@sbruens sbruens requested a review from a team as a code owner April 22, 2024 19:12
@sbruens
Copy link
Contributor Author

sbruens commented Apr 22, 2024

I made more changes. I think this is now ready for review.

daniellacosse
daniellacosse requested changes Apr 22, 2024
View reviewed changes
Copy link
Contributor

@daniellacosse daniellacosse left a comment •
edited
Loading

Choose a reason for hiding this comment

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

What are we planning to do with infrastructure files that weren't moved (i.e. client/infrastructure/electron/app_paths.ts)?

client/src/www/app/app.ts Show resolved Hide resolved
infrastructure/path_api.ts Show resolved Hide resolved
infrastructure/memory_storage.ts Show resolved Hide resolved
client/package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@daniellacosse
Copy link
Contributor

Ah, I just remembered - src/build needs to be moved into this module as well

fortuna
fortuna approved these changes Apr 23, 2024
View reviewed changes
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.

Looks great, thanks!

client/src/www/app/app.ts Show resolved Hide resolved
client/src/www/app/app.ts Show resolved Hide resolved
@daniellacosse
Copy link
Contributor

daniellacosse commented Apr 23, 2024 •
edited
Loading

Looks great, thanks!

There are a few things that need to moved into this module that haven't yet - @fortuna is that alright? I'm okay with splitting things up, but just trying to understand your ruleset.

daniellacosse
daniellacosse approved these changes Apr 23, 2024
View reviewed changes
Copy link
Contributor

@daniellacosse daniellacosse left a comment

Choose a reason for hiding this comment

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

I'm fine with handling the rest of the infrastructure files later

@sbruens sbruens merged commit ffc913c into master Apr 23, 2024
20 checks passed
@sbruens sbruens deleted the sbruens/infra branch April 23, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniellacosse daniellacosse daniellacosse approved these changes

@fortuna fortuna fortuna approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sbruens @daniellacosse @fortuna