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

Fix the error in case of no-cb require(["deps"]) #26

Merged
merged 2 commits into from Jun 8, 2015
Merged

Fix the error in case of no-cb require(["deps"]) #26

merged 2 commits into from Jun 8, 2015

Conversation

ghost
Copy link

@ghost ghost commented Jun 6, 2015

The require(["deps"]); fails as amdefine tries to call the callback unconditionally.

@jrburke
Copy link
Owner

jrburke commented Jun 7, 2015

Ah good idea. I just have one nit, to use braces with the if body, and a 4 space indentation to match the rest of the file.

If you want to update the pull request to that, that would be ideal. If you just wanted to report the issue and are OK with me doing the change myself on master with that style, then let me know and I will do it.

@ghost
Copy link
Author

ghost commented Jun 7, 2015

Just do it yourself, I edited in github itself, I don't know how / if ever I can fix it there.

James Burke wrote:

Ah good idea. I just have one nit, to use braces with the if body, and
a 4 space indentation to match the rest of the file.

If you want to update the pull request to that, that would be ideal.
If you just wanted to report the issue and are OK with me doing the
change myself on master with that style, then let me know and I will
do it.


Reply to this email directly or view it on GitHub
#26 (comment).

@ghost
Copy link
Author

ghost commented Jun 7, 2015

I did a change in bracing; as for spaces, I don't know what was the problem, I see them there in the diff...

jrburke added a commit that referenced this pull request Jun 8, 2015
Fix the error in case of no-cb `require(["deps"])`
@jrburke jrburke merged commit dbc2d67 into jrburke:master Jun 8, 2015
@jrburke jrburke added this to the 0.1.1 milestone Jun 8, 2015
@jrburke
Copy link
Owner

jrburke commented Jun 8, 2015

Merged and released 0.1.1 with the change.

@ghost
Copy link
Author

ghost commented Jun 9, 2015

Thanks, not it will work with optimizer's insertRequire ;-)

James Burke wrote:

Merged and released 0.1.1 with the change.


Reply to this email directly or view it on GitHub
#26 (comment).

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.

None yet

1 participant