[HOLD] Added AMD/CommonJS compatible definitions

[syavorsky]

No description provided.

[dcneiner]

@yavorskiy - Sorry for the delay in reviewing this request, I'll be looking to review it this week and provide feedback or merge as appropriate. Thanks for the time you put into this already!

[dcneiner]

@yavorskiy As I review the code, there seem to be a number of changes unrelated to adding Common JS/AMD support to the file. Can you please submit a new request that only adds CommonJS/AMD support, and then after that's merged we can look at the other fixes you put in place?

Also, I think I'd prefer "root" instead of "window" for your primary variable name in the factory function.

Thanks for the work you put into this already!

[syavorsky]

@dcneiner ok, happy to hear it's going to be merged.
I would still like to explain some changes you asked to remove, hope you gonna change you mind. Besides minor code linting the only material change there was tweaking $.ajaxSettings. Idea was to let function($) {} produce the module without implicitly changing $. This way $.* references are bound only in browser environment when it is invoked as a traditional jQuery plugin. For other environments it's just a module providing same interface and referring jQuery but not changing it.
I did notice mistake in my code though, where i try to redeclare settings which was defined at the top.
Let me know what you think, I will make all changes as needed.

[dcneiner]

@yavorskiy Ah, ok – that makes sense. I'd probably prefer the linting changes be made in a separate commit just so they aren't hidden in the CommonJS/AMD commit – but this makes sense. If you can fix the settings issue, and update it to work against master - that would be awesome.

[dcneiner]

@yavorskiy – I meant, I'd prefer the linting changes be separate but its fine to leave them in the CommonJS/AMD commit… no use creating extra work for something like that. Thank you!

[syavorsky]

@dcneiner ok, cool, I will clean it up.

[syavorsky]

@dcneiner sorry for taking that long. Please review the changes and let me know. I see one test failing (Request Data Matching: Correct data matching on request with arrays) when I run qunit locally but it wasn't passing even before I made changes.

[dcneiner]

@yavorskiy It was not my intention to let this sit so long – Thank you for your work on it, and I'll look to get it merged in early next week.

[pojka]

Any chance of this PR being accepted? Finding it very difficult to get mockjax working in a compiled AMD build which is a shame

[jakerella]

Our apologies again for the delayed response, we're just getting back into active development on this. I've looked through the changes and I think they should work just fine. Unfortunately, the repo that the changes came from no longer exists and so I cannot checkout that branch to run the tests.

@yavorskiy, am I missing something? Did you perhaps delete your fork? If so, we might take these changes and implement them ourselves unless you have any concerns there.

@dcneiner did you have any other comments on these changes?

[syavorsky]

Sorry, I am afraid it's deleted now, it seemed dead.

Sergii

On Fri, Aug 8, 2014 at 1:29 PM, Jordan Kasper notifications@github.com
wrote:

Our apologies again for the delayed response, we're just getting back into
active development on this. I've looked through the changes and I think
they should work just fine. Unfortunately, the repo that the changes came
from no longer exists and so I cannot checkout that branch to run the tests.

@yavorskiy https://github.com/yavorskiy, am I missing something? Did
you perhaps delete your fork? If so, we might take these changes and
implement them ourselves unless you have any concerns there.

@dcneiner https://github.com/dcneiner did you have any other comments
on these changes?

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

[jakerella]

@yavorskiy No problem,. what I might do is port this over to an issue and along with your code patch as an attachment, then we can do a new PR that includes proper tests and everything.

Sound ok?

[syavorsky]

Sure, sounds good
On Aug 9, 2014 10:57 AM, "Jordan Kasper" notifications@github.com wrote:

@yavorskiy https://github.com/yavorskiy No problem,. what I might do is
port this over to an issue and along with your code patch as an attachment,
then we can do a new PR that includes proper tests and everything.

Sound ok?

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

[prantlf]

I was in need for the module wrapper and wrote an alternative implementation with just the wrapping code and compliant to the UMD. I created PR #224 for your consideration, at first for the master branch only.

[jakerella]

Implemented in #244.