-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Webpack #3596
Webpack #3596
Conversation
.circleci/config.yml
Outdated
if [ "${CIRCLE_BRANCH}" = "development" ] && [ -n "$DEPLOY_HOST" ] && [ -n "$DEPLOY_USER" ] && [ -n "$DEPLOY_PASSWORD" ]; then | ||
sudo npm install -g grunt-cli | ||
grunt deploy --git-commit=$CIRCLE_SHA1 --ftp-host=$DEPLOY_HOST --ftp-user=$DEPLOY_USER --ftp-pass=$DEPLOY_PASSWORD | ||
if [ -n "$DEPLOY_HOST" ] && [ -n "$DEPLOY_USER" ] && [ -n "$DEPLOY_PASSWORD" ]; then |
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.
Removing the condition "${CIRCLE_BRANCH}" = "development" would result in having builds from any branch deployed via ftp as circle ci builds happen on any branch. Most of those deployment would be not desired in most cases (feature branches would overwrite the "nightly build"). Only from one agreed on branch deployments should happen (currently this was/is "development").
Btw, sorry, i owe you all some docu/howto on circle ci config ;)
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.
This was done temporarily to check ftp push feature in this branch .
This will restore before being merged
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.
ok, i'll keep this one open so we do not forget..
@bbert, until now githooks were added by module I fully support removing this outdated module as dep, so i tried to implement this functionality already as postinstall node script in What do you think about this approach @bbert ? @dsilhavy fyi |
Fine, I prefer this approach (postinstall) |
We can merge yours first, then I will update ts PR |
Ok, cool. I will pull out this out of the huge PR and make a separate/specific PR. This makes things faster. |
@bbert pls put: Update README.md to the "To complete:"-List ;) All this grunt stuff needs to be removed there |
@bbert As discussed, we have merged the git hook creation script and the postinstall instruction into development. |
OK thanks I'll do it |
@mlasak I updated script githooks.js, please check. |
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.
this last changes lgtm
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.
PR lgtm
Migrate bundler to webpack.
What has been done:
dev
task for watch modestart
task for running dev server with watch mode and hot reloadingpackage.json
in order to publish only dist folderTo complete: