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.

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

Add script to deploy web app #1723

Merged
merged 6 commits into from
Nov 18, 2019
Merged

Add script to deploy web app #1723

merged 6 commits into from
Nov 18, 2019

Conversation

belcherj
Copy link
Contributor

@belcherj belcherj commented Nov 15, 2019

Fix

Losing GitHub Actions we made the call to make deployment local rather than
adding another level of CI

Test

  1. Checkout branch
  2. npm run deploy staging
  3. Does it push to webapp-staging?
  4. Does staging.

Review

Only one developer is required to review these changes, but anyone can perform the review.

Release

  • Added script to deploy web app #1723

Losing GitHUb Actions we made the call to make deployment local rather than
adding another level of CI
@belcherj belcherj added this to the 1.12 milestone Nov 15, 2019
@belcherj belcherj self-assigned this Nov 15, 2019
@belcherj belcherj requested a review from a team November 15, 2019 20:19
Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

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

In a follow-up PR we can add instructions in the README if you want to get this merged, otherwise might be handy to do now.

Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

Typo aside, this seems to work. It did spit out this, which makes me wonder if some of the paths are broken:

Switched to a new branch 'webapp-staging'
rm: "." and ".." may not be removed

bin/deploy.sh Outdated Show resolved Hide resolved
bin/deploy.sh Outdated Show resolved Hide resolved
Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

Okay I'm not immediately seeing why but this script deletes my config-local.json.

bin/deploy.sh Outdated Show resolved Hide resolved
touch .gitignore
echo config-local.json >> .gitignore
git add --all
git commit -m "Build: $COMMIT_BRANCH [ci skip]"
Copy link
Member

Choose a reason for hiding this comment

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

if we're wanting to deploy a given branch why are we building this and making a commit to that branch?
do we not have a way to inject a different config for the different environments?

seems like we ought to be able to do it like this…

git checkout webapp-develop
git reset --hard develop

Does something in the build process push itself out to VIP Go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the head of the webapp, webapp-staging, and webapp-develop branches change VIP Go grabs the contents of the repo and pushes it to the server.

We do not have a way to inject a different config for the different environments.

I am not sure what the code suggestion achieves.

Copy link
Member

Choose a reason for hiding this comment

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

the code suggestion would change the HEAD of those branches and should trigger web hooks to fire. the branches effectively would work like tags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VIP Go doesn't build the app it only grabs what's in the folder. https://github.com/Automattic/simplenote-electron/tree/webapp

@belcherj belcherj merged commit fa3de32 into develop Nov 18, 2019
@belcherj belcherj deleted the add/deploy-script branch November 18, 2019 16:28
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