oppia / foundation-website

Repository for developing the new Oppia Foundation website.
Apache License 2.0
6 stars 33 forks source link

Use npm's package.json and npm install to manage node module dependencies. #75

Closed hoangviet1993 closed 6 years ago

hoangviet1993 commented 6 years ago

Prob overkill but changes are live at: https://oppia-foundation-test-server.appspot.com/ ( I will implement the CircleCI cache versioning in a different PR)

codecov-io commented 6 years ago

Codecov Report

Merging #75 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
+ Coverage   95.94%   95.95%   +0.01%     
==========================================
  Files          18       18              
  Lines         345      346       +1     
==========================================
+ Hits          331      332       +1     
  Misses         14       14
Impacted Files Coverage Δ
app/config.py 100% <ø> (ø)
app/core/tests/unit_test_base.py 71.42% <ø> (ø)
app/core/domain/email_manager.py 100% <ø> (ø)
app/core/controllers/base.py 100% <ø> (ø)
app/core/controllers/outgoing_emails_test.py 100% <ø> (ø)
app/core/domain/email_manager_test.py 100% <ø> (ø)
app/core/utility/string_validator_test.py 100% <ø> (ø)
app/core/utility/constants_test.py 100% <ø> (ø)
app/core/utility/string_validator.py 100% <ø> (ø)
app/core/platform/gae_email_services_test.py 100% <ø> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c59250a...16e250f. Read the comment docs.

hoangviet1993 commented 6 years ago

@seanlip

  1. This is to help keep apps' dependencies separated from another. I.e node_modules folder of /foundation-website should stay inside in its root folder because only /foundation-website should have access to them. If I was to bring it outside, I would need to create another sub-folder to prevent sharing that folder with main oppia app. Grunt will also do this, here is a SO post that seems to be supporting my idea. https://stackoverflow.com/a/15523895

  2. I have been wanting to move app.yaml inside /static for a while now. This is simply because of what I currently know about app.yaml honestly. From what I understand, app.yaml will upload to GCP all files in the same subfolder it is in as well as files in the subfolders below it. So basically, it should go into the "public" folder where all files are sent to the client. The root folder has all these dot config files, dot config folder and potentially the node_modules folder. These files should not be uploaded / made public at any point. Hope I made my points clear enough here.

Re: subfolder for node_modules, can you provide a high level directory tree of what you want to see when /node_modules resides within the root directory?

seanlip commented 6 years ago

Ah, I see, thanks for the explanations! /cc @LanJosh since he is also looking into package.json stuff for the main Oppia repo.

I think maybe we can go for something like (in the root folder):

What do you think?

hoangviet1993 commented 6 years ago

@seanlip sounds great to me 👍

hoangviet1993 commented 6 years ago

@seanlip PTAL!

seanlip commented 6 years ago

Hi @hoangviet1993 -- merge conflicts?

hoangviet1993 commented 6 years ago

@seanlip PTAL