-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Allow additional matchers to be added without monkey patching prototype. #14
Conversation
Submitted ember-cli/ember-cli-qunit#77 to use this API. |
@@ -53,9 +82,14 @@ define("ember-cli/test-loader", | |||
new TestLoader().loadModules(); | |||
}; | |||
|
|||
TestLoader.addModuleIncludeMatcher = addModuleIncludeMatcher; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you expose the add functions in 2 different ways by both adding them to the TestLoader function here, as well as returning the function directly in the return object below (90-91). I'm ok with it, but just curious why both were done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just indecisiveness 😝
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess adding it to TestLoader will save us a line of code on the other end, instead of setting the function to a separate variable off the module, we can just invoke it directly on TestLoader. (I'm talking about the other pull request: https://github.com/ember-cli/ember-cli-qunit/pull/77/files line 6 and 12-13). Not a big deal at all though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, I went the other way. Modules 4 LYFE!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Currently, `TestLoader.prototype.shouldLoadModule` is monkey patched by ember-cli-qunit / ember-cli-mocha to add their own custom rules for matching modules. This change allows additional addons to add their own custom matchers (for example adding additional extension types) or to suppress a certain subset of modules (for example JSHint or JSCS tests).
ba78280
to
eecfbc1
Compare
🚢shipit, thx for cranking this out |
Allow additional matchers to be added without monkey patching prototype.
Currently,
TestLoader.prototype.shouldLoadModule
is monkey patched by ember-cli-qunit / ember-cli-mocha to add their own custom rules for matching modules.This change allows additional addons to add their own custom matchers (for example adding additional extension types) or to suppress a certain subset of modules (for example JSHint or JSCS tests).