Skip to content

Conversation

@saponifi3d
Copy link
Contributor

Quick fix for require vs requireAMD in the shared/app.js

  • need to deal with the callback functions for requireAMD

@saponifi3d saponifi3d mentioned this pull request Mar 26, 2015
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main problem with the signature – array + callback vs string + return.

@purusho Did it work for you in 1.0.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hah yer too quick, yeah, i did it really quickly and went "crap, this won't work..." i renamed this to be a work in progress and added a checkbox for the task to make this a callback. i'd be realllyyy surprised if this worked at all before. This will probably require more work to get things to work, and i need to setup a requirejs env to really test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we need to think it through.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was working on 1.0.1 with ClientRouter = require('app/router');.

I don't think PR is going to fix the issue

@saponifi3d saponifi3d closed this Mar 26, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.89% when pulling d36d957 on require-amd-clientrouter into a450430 on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants