Skip to content

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

Merged
moloko merged 2 commits into
masterfrom
enhancement/929
Feb 16, 2016
Merged

enhancement/929: remove core require mappings "coreJS" "coreViews"#930
moloko merged 2 commits into
masterfrom
enhancement/929

Conversation

@oliverfoster

@oliverfoster oliverfoster commented Jan 31, 2016

Copy link
Copy Markdown
Member
  • 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

moloko commented Feb 1, 2016

Copy link
Copy Markdown
Contributor

+1

@taylortom

Copy link
Copy Markdown
Member

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

@oliverfoster

Copy link
Copy Markdown
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
Copy Markdown
Member

+1

1 similar comment
@taylortom

Copy link
Copy Markdown
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