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

Fix building with TS config #2372

Merged
merged 5 commits into from
Nov 19, 2024
Merged

Fix building with TS config #2372

merged 5 commits into from
Nov 19, 2024

Conversation

sodic
Copy link
Contributor

@sodic sodic commented Nov 15, 2024

Fixes #2368

sodic
sodic commented Nov 15, 2024
View reviewed changes
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/src/Wasp/Util/Json.hs
Comment on lines +35 to +36
writeJsonValue :: Path' Abs (File f) -> Value -> IO ()
writeJsonValue file = IOUtil.writeFileBytes file . encode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about whether I should put this here or in IOUtil, but I decided this makes more sense in the end.

infomiho
infomiho approved these changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@infomiho infomiho left a comment

Choose a reason for hiding this comment

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

LGTM, I'd just write a comment above the lens stuff (the intention of the code)

waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
Martinsos
Martinsos reviewed Nov 19, 2024
View reviewed changes
Copy link
Member

@Martinsos Martinsos left a comment

Choose a reason for hiding this comment

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

@sodic looks good! This actually turned out to be quite simple merely a bit of manipulation on json files and that is it really. Code is also quite simple due to lenses, couple of lines.

Most stuff I commented are minor improvements in code cleanliness, but one is actually a potentiall bug in hiding (although not likely to happen), which is why I didn't approve it yet. Let's fix that and whatever code improvements you like and then we can do it.

waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/cli/src/Wasp/Cli/Command/Build.hs Outdated Show resolved Hide resolved
waspc/src/Wasp/Util/IO.hs Show resolved Hide resolved
waspc/src/Wasp/Util/Json.hs Outdated Show resolved Hide resolved
Martinsos
Martinsos approved these changes Nov 19, 2024
View reviewed changes
Copy link
Member

@Martinsos Martinsos left a comment

Choose a reason for hiding this comment

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

LGTM!

@sodic sodic merged commit 1e62987 into main Nov 19, 2024
6 checks passed
@sodic sodic deleted the filip-fix-ts-config-deploy branch November 19, 2024 18:31
@sodic sodic mentioned this pull request Nov 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@infomiho infomiho infomiho approved these changes

@Martinsos Martinsos Martinsos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix TS Config build
3 participants
@sodic @Martinsos @infomiho