Skip to content

Switch dev-ui command to use npm run dev#112

Merged
nick1udwig merged 5 commits intomasterfrom
dr/gus-dev-ui-fix
Mar 30, 2024
Merged

Switch dev-ui command to use npm run dev#112
nick1udwig merged 5 commits intomasterfrom
dr/gus-dev-ui-fix

Conversation

@dr-frmr
Copy link
Copy Markdown
Contributor

@dr-frmr dr-frmr commented Mar 20, 2024

Frontend build command should use a fast debug build unless developer specifically wants to build for release. To this end, we switch dev-ui to use npm run dev unless the --release flag is passed, in which case it will use npm start.

kit b will remain unchanged.

@dr-frmr dr-frmr requested a review from nick1udwig March 20, 2024 17:26
@nick1udwig
Copy link
Copy Markdown
Member

Comment in general before diving in to the review: we don't currently enforce rustfmt on this repo and I don't want to until I can look into how to make it not make certain cases less readable IMO, see #74

However, I know you and some others have editors configured to auto-rustfmt, and that makes it harder to review PRs because there are lots of lines changed that are just formatting. Is there a way you can toggle this behavior so you are not making lots of formatting changes here/in future?

Copy link
Copy Markdown
Member

@nick1udwig nick1udwig left a comment

Choose a reason for hiding this comment

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

Added some comments. Please address, then good to merge. Thanks for this work!

Comment thread src/dev_ui/mod.rs Outdated
Comment thread src/main.rs Outdated
Comment thread src/main.rs Outdated
Copy link
Copy Markdown
Contributor

@gusapps gusapps left a comment

Choose a reason for hiding this comment

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

Tested this with fake-node and it does what was intended 👍

@nick1udwig nick1udwig merged commit 67c6b6f into master Mar 30, 2024
@nick1udwig nick1udwig deleted the dr/gus-dev-ui-fix branch March 30, 2024 23:34
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