Skip to content
This repository has been archived by the owner. It is now read-only.

refactor(app): remove unused amd requires #2448

Merged
merged 1 commit into from May 22, 2015
Merged

Conversation

@riadhchtara
Copy link
Contributor

@riadhchtara riadhchtara commented May 20, 2015

Fixes #2392

@riadhchtara
Copy link
Contributor Author

@riadhchtara riadhchtara commented May 20, 2015

@vladikoff : i don't why my previous pull request was closed. Cold you please have look to the pr quickly before some new conflicts appear?
Thanks

@coveralls
Copy link

@coveralls coveralls commented May 20, 2015

Coverage Status

Coverage remained the same at 98.34% when pulling 99fe4d2 on riadhchtara:amd into 83d3fb1 on mozilla:master.

@vladikoff vladikoff self-assigned this May 20, 2015
@@ -7,7 +7,6 @@
define([
'underscore',
'cocktail',
'canvasToBlob',

This comment has been minimized.

@vladikoff

vladikoff May 21, 2015
Contributor

We cannot remove this, polyfill for older browsers:

this.canvas.toBlob(function (data) {

Needs exceptsPaths

This comment has been minimized.

@riadhchtara

riadhchtara May 21, 2015
Author Contributor

ah ok

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented May 21, 2015

This needs to be updated: #2448 (comment)

Secondary:

As part of a separate PR (to minimize diff) add the

tests/functional/**/*.js

files to amdcheck as well.

Report for those:

➜  fxa-content-server git:(riadhchtara-amd) grunt amdcheck
Running "amdcheck:app" (amdcheck) task
tests/functional/404.js (1 module)
Unused paths: intern/chai!assert

tests/functional/500.js (1 module)
Unused paths: intern/chai!assert

tests/functional/avatar.js (1 module)
Unused paths: app/scripts/lib/constants

tests/functional/bounced_email.js (1 module)
Unused paths: intern/chai!assert

tests/functional/delete_account.js (1 module)
Unused paths: intern/chai!assert

tests/functional/firefox/functional_firefox.js (1 module)
Unused paths: ./same_account

tests/functional/firefox/same_account.js (1 module)
Unused paths: intern/chai!assert

tests/functional/force_auth.js (1 module)
Unused paths: intern/node_modules/dojo/Deferred, tests/lib/restmail

tests/functional/oauth_force_email.js (1 module)
Unused paths: require

tests/functional/oauth_iframe.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/oauth_preverified_sign_up.js (1 module)
Unused paths: intern/node_modules/dojo/node!xmlhttprequest, app/bower_components/fxa-js-client/fxa-client, tests/lib/restmail

tests/functional/oauth_reset_password.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/oauth_sign_up.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/oauth_sign_up_verification_redirect.js (1 module)
Unused paths: intern, intern/node_modules/dojo/node!xmlhttprequest, tests/lib/restmail

tests/functional/oauth_sync_sign_in.js (1 module)
Unused paths: intern/node_modules/dojo/node!leadfoot/helpers/pollUntil

tests/functional/oauth_webchannel.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/oauth_webchannel_keys.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/pages.js (1 module)
Unused paths: intern/chai!assert

tests/functional/pp.js (1 module)
Unused paths: intern/chai!assert

tests/functional/settings_common.js (1 module)
Unused paths: intern/chai!assert, app/scripts/lib/constants

tests/functional/sign_in.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/sign_in_cached.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/sign_up.js (1 module)
Unused paths: intern/node_modules/dojo/node!leadfoot/helpers/pollUntil

tests/functional/sync_reset_password.js (1 module)
Unused paths: tests/lib/restmail

tests/functional/sync_settings.js (1 module)
Unused paths: intern/chai!assert, app/scripts/lib/constants

tests/functional/sync_sign_in.js (1 module)
Unused paths: intern/chai!assert, intern/node_modules/dojo/node!leadfoot/helpers/pollUntil

tests/functional/sync_sign_up.js (1 module)
Unused paths: intern/node_modules/dojo/node!leadfoot/helpers/pollUntil

tests/functional/tos.js (1 module)
Unused paths: intern/chai!assert


Total unused paths: 36 in 28 files.
Total unused dependencies: 35 in 27 files.
Total processed files: 288
Warning: Task "amdcheck:app" failed. Use --force to continue.

I will create a new issues after this PR gets merged to track the secondary task above.

@vladikoff vladikoff removed their assignment May 21, 2015
@riadhchtara riadhchtara force-pushed the riadhchtara:amd branch from 99fe4d2 to fe05dc6 May 21, 2015
@riadhchtara
Copy link
Contributor Author

@riadhchtara riadhchtara commented May 21, 2015

@vladikoff : thanks for the review.
How about this new patch?
I will work on the new tests/functional later, when this PR is merged as you said.

@coveralls
Copy link

@coveralls coveralls commented May 21, 2015

Coverage Status

Coverage remained the same at 98.24% when pulling fe05dc6 on riadhchtara:amd into 398f70a on mozilla:master.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented May 22, 2015

@vladikoff vladikoff modified the milestones: train 38, train 39 May 22, 2015
@riadhchtara riadhchtara force-pushed the riadhchtara:amd branch from fe05dc6 to 958e386 May 22, 2015
@riadhchtara
Copy link
Contributor Author

@riadhchtara riadhchtara commented May 22, 2015

@vladikoff : hi ! I have just done! the rebase, could you p0lease review it quickly!
there are so many files changed here so there is a big probability that a conflict appearers.
Thanks

@coveralls
Copy link

@coveralls coveralls commented May 22, 2015

Coverage Status

Coverage remained the same at 98.25% when pulling 958e386 on riadhchtara:amd into 0860054 on mozilla:master.

@vladikoff vladikoff self-assigned this May 22, 2015
vladikoff added a commit that referenced this pull request May 22, 2015
refactor(app): remove unused amd requires r=vladikoff
@vladikoff vladikoff merged commit e4a52ab into mozilla:master May 22, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 98.25%
Details
@vladikoff vladikoff removed the backlog label May 22, 2015
@pdehaan
Copy link
Contributor

@pdehaan pdehaan commented May 23, 2015

Great job, thanks again @riadhchtara!

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented May 23, 2015

Yeah this should be really useful!

@riadhchtara
Copy link
Contributor Author

@riadhchtara riadhchtara commented May 23, 2015

YES!!! it landed after 4 or 5 rebases.
@vladikoff @vladikoff thanks for you too guys

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants