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

enhancement/929: remove core require mappings "coreJS" "coreViews" #930

Merged
merged 2 commits into from
Feb 16, 2016

Conversation

oliverfoster
Copy link
Member

@oliverfoster oliverfoster commented Jan 31, 2016

  • removed explicit conversion of module declarations to "coreJS" style from "core/js" (+models, views etc)
  • remapped current require("coreJS/adapt) style to "core/js/adapt" in grunt
  • remapped current require("coreJS/adapt) style to "core/js/adapt" in browser
    remove core require mappings "coreJS" "coreViews" #929

actions to follow:

  • convert all old require declarations from "coreJS" style to new "core/js/" style

should be unobtrusive as the old style can be remapped to the new style until we've converted everything.

@moloko
Copy link
Contributor

moloko commented Feb 1, 2016

+1

@taylortom
Copy link
Member

+1 for concept (need to test for backwards-compatibility)

@oliverfoster
Copy link
Member Author

if you replace all the src/core/js/app.js "coreJS/adapt" with "core/js/adapt" you'll get the idea about backwards compatibility. it should just work with either for the time being. it makes "coreJS" map to 'core/js' rather than defining all the modules as "coreJS/..etc.." they are defined as "core/js/..etc..". we can remove this new mapping of coreJS > core/js in v3 and have just core/js. they will both resolve to the same place for the moment.

@dancgray
Copy link
Member

+1

1 similar comment
@taylortom
Copy link
Member

+1

moloko added a commit that referenced this pull request Feb 16, 2016
enhancement/929: remove core require mappings "coreJS" "coreViews"
@moloko moloko merged commit 6045dc9 into master Feb 16, 2016
@moloko moloko deleted the enhancement/929 branch February 16, 2016 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants