oppia / foundation-website

Repository for developing the new Oppia Foundation website.

Apache License 2.0

6 stars 33 forks source link

Added Google Analytics event tracking for volunteers and Partnerships #97

Closed dchen97 closed 5 years ago

dchen97 commented 5 years ago

Added in code necessary to track events related to people submitting the Volunteer and Partnership interest forms.

dchen97 commented 5 years ago

@seanlip does everything look good with the indentation to push this?

seanlip commented 5 years ago

Yes, this all looks fine -- but could you please look into why the linter etc. checks are all failing?

seanlip commented 5 years ago

/cc @oppia/dev-workflow-team @apb7

anubhavsinha98 commented 5 years ago

Hi @dchen97, the linting tests are failing due to wrong-import-order

anubhavsinha98 commented 5 years ago

Here is the log. Screenshot from 2019-03-29 03-43-46

anubhavsinha98 commented 5 years ago

Both the integrations Travis as well as Circle are running the lint tests. Fixing the lint test would result to fix both the CIs. Thanks!

dchen97 commented 5 years ago

@anubhavsinha98 Do you know why these linter errors are popping up in files I didn't touch in this PR? Also, I addressed all the errors I could see, but it's still saying that the linter tests are failing by exiting with the code 1 and no description of how to resolve that.

anubhavsinha98 commented 5 years ago

Hi @dchen97, yeah I was also amazed that how it was showing the wrong-import-order, as the earlier PRs were passing the lint test. I will take a look and tell you. Thanks!

seanlip commented 5 years ago

Hi @anubhavsinha98 -- friendly ping?

anubhavsinha98 commented 5 years ago

Hi, @seanlip was looking at error. Actually, my exams are going one. Will provide you with an update within 1-2 days. Please, tell me if it's fine? Thanks.

seanlip commented 5 years ago

Yes, of course. Good luck with exams, and thanks for the update!

anubhavsinha98 commented 5 years ago

Thanks Sean!

On Wed, Apr 3, 2019, 1:01 PM Sean Lip notifications@github.com wrote:

Yes, of course. Good luck with exams, and thanks for the update!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/oppia/foundation-website/pull/97#issuecomment-479373702, or mute the thread https://github.com/notifications/unsubscribe-auth/AdI9mLTdgJaY3U9FuF20FCAJh439UyD3ks5vdFjogaJpZM4batly .

anubhavsinha98 commented 5 years ago

Hi @dchen97, I would suggest reverting the import order changes that you made. And update your branch with the master.

codecov-io commented 5 years ago

Codecov Report

Merging #97 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #97   +/-   ##
=======================================
  Coverage   96.41%   96.41%           
=======================================
  Files          21       21           
  Lines         390      390           
=======================================
  Hits          376      376           
  Misses         14       14

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 7c5c4aa...eaf45cb. Read the comment docs.

dchen97 commented 5 years ago

@seanlip Are there any other changes you need before we can merge this in?