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

Hussainakram/Update dependencies #470

Merged
merged 5 commits into from
Jun 19, 2020
Merged

Conversation

hussainakram
Copy link
Member

@hussainakram hussainakram commented Jun 15, 2020

Summary

update all dependencies

Implementation Tasks


This change is Reviewable

@hussainakram hussainakram requested a review from justin808 June 15, 2020 12:42
@hussainakram hussainakram self-assigned this Jun 15, 2020
Copy link
Member

@justin808 justin808 left a comment

Choose a reason for hiding this comment

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

If this all works good locally, let's merge this. Did you manually test?

I'm surprised that the translation parts work.

And I'm also surprised that we didn't have to change some of the registered react component definitions.

Reviewed 11 of 11 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @hussainakram)


.gitignore, line 42 at r1 (raw file):

client/app/libs/i18n/translations.json
client/app/libs/i18n/default.js
client/app/libs/i18n/default.json

Changing from js to json is optional and recommended.

If we switch, delete the unused entries.


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

services:
  - docker
  - postgresql

why did this need to get added?


Gemfile.lock, line 4 at r1 (raw file):

  remote: https://rubygems.org/
  specs:
    actioncable (5.2.4.3)

this should be going to v6, but that can be in a separate PR.


Gemfile.lock, line 374 at r1 (raw file):

RUBY VERSION
   ruby 2.7.0p0

this should be 2.7.1


app/assets/config/manifest.js, line 1 at r1 (raw file):

{}

why is this file getting added?


client/package.json, line 39 at r1 (raw file):

    "build:test": "rm -rf ../public/webpack/test && NODE_ENV=test yarn run build:client && NODE_ENV=test yarn run build:server",
    "build:production": "rm -rf ../public/webpack/production && NODE_ENV=production yarn run build:production:client && yarn run build:production:server",
    "hot-assets": "NODE_ENV=development babel-node server-rails-hot.js",

this is way outdated


client/package.json, line 47 at r1 (raw file):

    "axios": "^0.17.1",
    "babel-cli": "^6.26.0",
    "babel-core": "^6.26.0",

very old babel, but this can be updated by switching to the spec/dummy setup for the webpack config.


client/package.json, line 97 at r1 (raw file):

    "turbolinks": "^5.0.3",
    "url-loader": "^0.6.2",
    "webpack": "^3.8.1",

old version of webpack

.travis.yml Outdated
@@ -1,10 +1,11 @@
language: ruby

rvm:
- 2.5.3
- 2.7.0
Copy link
Member

Choose a reason for hiding this comment

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

2.7.1

Copy link
Member

Choose a reason for hiding this comment

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

???

Copy link
Member Author

@hussainakram hussainakram 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: 6 of 10 files reviewed, 9 unresolved discussions (waiting on @hussainakram and @justin808)


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

Previously, justin808 (Justin Gordon) wrote…

2.7.1

Done


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

Previously, justin808 (Justin Gordon) wrote…

why did this need to get added?

Mentioned in the PR description


Gemfile.lock, line 374 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

this should be 2.7.1

Done


app/assets/config/manifest.js, line 1 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

why is this file getting added?

Mentioned in the PR description


client/package.json, line 97 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

old version of webpack

I'm planning to do it in a separate PR as that requires dealing with some deprecation warnings and some other changes mentioned here: https://stackoverflow.com/a/49213048/5415898

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