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

chore: add a utility script to make development easier #512

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −0
Open

chore: add a utility script to make development easier #512

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"build:pre": "bash ./scripts/pre.sh",
"release": "changelog --link-url https://github.com/quipucords/quipucords-ui.git",
"start": "export PROTOCOL=http; export HOST=127.0.0.1; export PORT=${PORT:-3000}; export MOCK_PORT=${MOCK_PORT:-3030}; run-p -l api:dev start:js start:open",
"start:using-server": "export PROTOCOL=${QUIPUCORDS_SERVER_PROTOCOL:-https}; export HOST=${QUIPUCORDS_SERVER_HOST:-127.0.0.1}; export PORT=${PORT:-3000}; export MOCK_PORT=${QUIPUCORDS_SERVER_PORT:-9443}; run-p -l start:js start:open",
Copy link
Contributor

@cdcabrera cdcabrera Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

some level of alteration is needed, a couple of choices for you...

Option 1: highly recommend

  • combining the original and new script. reason, currently, the new script is customized for a single developer where as the original start script was meant to be expandable for multiple use cases, modifying the original would be natural

    technically the npm-run-all is just looking for strings to run npm scripts so replacing it with an exported empty command means it skips it...

       "start": "export PROTOCOL=${DEV_SERVER_PROTOCOL:-http}; export HOST=${DEV_SERVER_HOST:-127.0.0.1}; export PORT=${DEV_SERVER_PORT:-3000}; export MOCK_PORT=${DEV_SERVER_MOCK_PORT:-3030}; export SERVER_SCRIPT=${DEV_SERVER_SCRIPT:-api:dev}; run-p -l $SERVER_SCRIPT start:js start:open",

Option 2: if the unique script is kept...

  • start:using-server should be renamed...
    • reason, description is generic. technically the original script is also running various servers
    • recommend something like start:custom-server or something direct like start:custom since having to continuously type something with a hyphen gets tedious, for those who still reboot/logout their computers =) (there are regrets around the integration-dev script naming)

And an additional recommendation, but not necessary

  • the prefix QUIPUCORDS_SERVER_ should probably be shortened
    • reason, if the values using QUIPUCORDS_SERVER_ are expected to be typed out by follow up developers we could be kind by shorten them. But, if there's another script exporting these values this is less of a concern.

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'll try to work with option 1! Better not to have unecessary extra commands.

now, about QUIPUCORDS_ prefixes being long, this is a team decision. We want ALL env vars configuring quipucords to have this prefix, even if it makes them longer.

"start:js": "export NODE_ENV=development; weldable -l ts -x ./config/webpack.dev.js",
"start:open": "xdg-open $PROTOCOL://$HOST:$PORT/ || open $PROTOCOL://$HOST:$PORT/",
"start:stage": "export PROTOCOL=http; export HOST=127.0.0.1; export PORT=${PORT:-3000}; export MOCK_PORT=${MOCK_PORT:-8000}; run-p -l api:stage start:js start:open",
Expand Down
Loading