Skip to content

Add AMD support#15

Merged
hilios merged 1 commit intohilios:v1.0.1from
FagnerMartinsBrack:amd
Oct 1, 2013
Merged

Add AMD support#15
hilios merged 1 commit intohilios:v1.0.1from
FagnerMartinsBrack:amd

Conversation

@FagnerMartinsBrack
Copy link
Copy Markdown
Contributor

No description provided.

@FagnerMartinsBrack
Copy link
Copy Markdown
Contributor Author

haha forgot to add the pull request 2 days ago, just let me know your thoughts regarding this

hilios added a commit that referenced this pull request Oct 1, 2013
@hilios hilios merged commit f892ecb into hilios:v1.0.1 Oct 1, 2013
@hilios
Copy link
Copy Markdown
Owner

hilios commented Oct 1, 2013

LGTM, thanks man!

@hilios
Copy link
Copy Markdown
Owner

hilios commented Oct 25, 2013

@FagnerMartinsBrack remember I told you about a plugin refactor? Well I managed to get time this week to accomplish this task.

The amount of updates where so large that I finished by almost rewriting the plugin, so I called this revision 2.0.0 instead of 1.0.1. I will launch both in the same time, because the ~2.0 does not support jquery 1.6 and ~1.0 does.

I totally understand your point about testing the AMD support, I agree that this shouldn't be included has a test case, but, has a test scenario I think is worth it. This means, see if all current test cases are being executed when RequireJS is included.

In this refactor I tried to cover as many jQuery versions as I could, for each one I created a scenario for it. I also received an issue about the plugin is not working with bootstrap, so I generated another scenario including bootstrap.js as well. See where I am coming?

If you could help me with this issue it would be great, I already created an HTML (the scenario) to run the plugin with RequireJS, but I really dunno how to make everything work together. You can see code at branch v2.0.0 under test/scenario-requirejs.html.

BR,

@FagnerMartinsBrack
Copy link
Copy Markdown
Contributor Author

Let me see if I understand: You just want to make the whole test suite run using requireJS instead of creating an specific test for that matter, is that correct?
The only issue will be the time lost running the whole suite again for a simple feature, but it may be worth it.

@FagnerMartinsBrack
Copy link
Copy Markdown
Contributor Author

Actually is not that simple, you need to call require(["jquery", "jquery.countdown"], function( $ ) {}) on each test you are going to use it, otherwise it should not load.
It is enough to create a single file with a single call of requireJS, there is no need for running all the test since requireJS matters only for loading and that is it.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants