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

requireJS for version 2.0.0 #18

Merged
merged 1 commit into from
Nov 1, 2013
Merged

requireJS for version 2.0.0 #18

merged 1 commit into from
Nov 1, 2013

Conversation

FagnerMartinsBrack
Copy link
Contributor

Few notes:
Due to some error with npm install -d while installing the qunit grunt plugin I had to downgrade it to the version 1.0.0 to make it work.
I will comment each line explaining the changed

@@ -0,0 +1,9 @@
module('requireJS');

asyncTest('test requireJS support', function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we create a single test for requireJS instead of running the whole suite, which would be pretty useless once we agree the support is working

@FagnerMartinsBrack
Copy link
Contributor Author

After reviewing this I realized it may not be necessary to create a separate file or hide the jquery global.
If I use the same code in the original test file, without import the plugin by the script tag, I may be able to get this to work without creating so many files.

@FagnerMartinsBrack
Copy link
Contributor Author

By the way, can't you use #qunit-fixture instead of $dom = $('<div id="clock" />').appendTo('body'); in the tests file?

@FagnerMartinsBrack
Copy link
Contributor Author

Everything in this pull request should be used under wtfpl public license

@hilios
Copy link
Owner

hilios commented Nov 1, 2013

I agree with you about using the #qunit-fixture, but I will need to create the element on the fly, I will just append it to the right element, and let qunit do the clean-up.

By the way, this is much a cleaner way to test the requirejs support, great job! I will merge this right away.

Thanks,

@FagnerMartinsBrack
Copy link
Contributor Author

but I will need to create the element on the fly

I don't understand. Which element?

@hilios
Copy link
Owner

hilios commented Nov 1, 2013

Never mind!

hilios added a commit that referenced this pull request Nov 1, 2013
@hilios hilios merged commit 246dcff into hilios:v2.0.0 Nov 1, 2013
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