-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Fix #5623 Move server launch to python #5657
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scripts/start.sh
Outdated
($NODE_PATH/bin/node $NODE_MODULE_DIR/gulp/bin/gulp.js start_devserver --prod_env=$FORCE_PROD_MODE --gae_devserver_path=$GOOGLE_APP_ENGINE_HOME/dev_appserver.py --clear_datastore=$CLEAR_DATASTORE_ARG --enable_console=$ENABLE_CONSOLE_ARG)& | ||
|
||
if ! [[ "$FORCE_PROD_MODE" == "True" ]]; then | ||
($NODE_PATH/bin/node $NODE_MODULE_DIR/gulp/bin/gulp.js watch)& |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deindent by 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@LanJosh Do we still need gulp at all now that we are using gcloud's dev_appserver.py? |
@hoangviet1993 There is still the watch functionality being used by gulp currently. I believe I'll be able to remove it in a later PR and replace it with something already in our stack. |
Thanks @LanJosh! LGTM. |
Explanation
Fix #5623 Removes the piping in gulp to start the app devserver. Retains the watch functionality in gulp.
Checklist
python scripts/pre_commit_linter.py
andbash scripts/run_frontend_tests.sh
.