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

Migrate site deployment to Firebase #9

Merged
merged 17 commits into from
Feb 15, 2019
Merged

Migrate site deployment to Firebase #9

merged 17 commits into from
Feb 15, 2019

Conversation

critzo
Copy link

@critzo critzo commented Feb 14, 2019

This PR replaces the Travis deployment to AWS with deployment to Google Firebase. Only the production site is added to this PR in the in the interest of moving off AWS completely. All changes in this PR can be previewed here: https://mlab-speedtest.firebaseapp.com

All currently completed translations from Transifex are also added with this PR. Supported languages other than English will now include:

  • az - Azerbaijani
  • zh - Chinese
  • nl - Dutch
  • fr - French
  • de - German
  • el - Greek
  • hi - Hindi
  • id - Indonesian
  • fa - Persian
  • ru - Russian
  • es - Spanish

@nkinkade PTAL?


This change is Reviewable

@critzo critzo requested a review from nkinkade February 14, 2019 19:46
Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

Once comment on the .travis.yml file, and a general question about whether we should be including any "lock" and "cache" files in the repo. For example, the yarn.lock file says it is autogenerated (and it's very large). Is this a file that needs to be in version control?

Reviewed 39 of 43 files at r1.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @critzo and @nkinkade)


.travis.yml, line 17 at r1 (raw file):

  on:
    repo: m-lab/mlab-speedtest
    all_branches: true

I think all_branches can be removed, since we are dealing with tag here, not a branch.


yarn.lock, line 1 at r1 (raw file):

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.

Should this auto-generated files be included in version control?

@critzo
Copy link
Author

critzo commented Feb 14, 2019

I'll have to test without the yarn file in the repo. I think we can get away with dropping it but will check. It's only there to get around some issues with bower being deprecated. The reality is that this app needs a major update but getting it migrated off AWS will have to precede that since we likely won't get to that for a bit. I'll make another commit and ping you back. Thanks @nkinkade

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

How is yarn.lock generated? Is package-lock.json generated too?

Reviewed 1 of 3 files at r2.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @critzo)

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @critzo)


.travis.yml, line 17 at r1 (raw file):

Previously, nkinkade wrote…

I think all_branches can be removed, since we are dealing with tag here, not a branch.

condition can be removed too.

@critzo
Copy link
Author

critzo commented Feb 15, 2019

@nkinkade The app is dated enough that it is using bower to package node modules. It needs to be updated in the future after it is migrated. Just to get the app updated and working, it was necessary to do an initial migration step, as described here. yarn.lock was generated when I used the bower-away node module to repackage and update the node modules in the app. bower-away uses yarn and generates that file, and I assume package-lock.json as well.

I would recommend that we get this app hosting migrated and deal with refactoring it later. Is that ok? Realistically, we need to use something like webpack and that will take more time.

@critzo
Copy link
Author

critzo commented Feb 15, 2019


.travis.yml, line 17 at r1 (raw file):

Previously, nkinkade wrote…

condition can be removed too.

removed in most recent commit.

@critzo
Copy link
Author

critzo commented Feb 15, 2019


yarn.lock, line 1 at r1 (raw file):

Previously, nkinkade wrote…

Should this auto-generated files be included in version control?

This file does need to be in version control

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

@critzo: Thanks for looking into all these. LGTM!

Reviewed 2 of 43 files at r1, 1 of 1 files at r3.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @nkinkade)

@critzo critzo merged commit 04bec81 into master Feb 15, 2019
@critzo critzo deleted the migrate-firebase branch February 15, 2019 17:05
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.

2 participants