-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Custom reporter integration #3064
Changes from 1 commit
98d01b5
1215c0b
b3196ac
44f6550
aa548c9
268a640
e79ae0c
2ee53ce
073d6e9
6a430de
329df49
7cc7aea
138bec3
6da8dad
352380b
1a5ac39
124ee97
d067e59
302b672
a9e9fce
31b51e3
75e39d1
12e5382
08bd0cf
8bf1bb8
06e7103
32dcff4
45656ad
15abe4e
76ebadb
e158496
80c88db
4a33e50
352f219
03c2045
8dd5ef3
27c5533
cebb62e
e36d442
593040c
c94c993
fc8a4d3
cfdcccf
552d0e2
7d903c9
1aa5d54
599c6ed
b5aa966
d1cf92e
23a7610
dd2d652
33640be
937a3c9
6dc65fb
492e0de
8c486e7
e328f93
bfad068
f4ed436
3b154d2
f6fa7bb
9ac6de0
e093125
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -364,11 +364,8 @@ class TestRunner { | |
|
||
_setupReporters() { | ||
const config = this._config; | ||
|
||
if (config.reporters) { | ||
this._addCustomReporters(config.reporters); | ||
} | ||
|
||
|
||
|
||
// Default Reporters are setup when | ||
// noDefaultReporters is false, which is false by default | ||
// and can be set to true in configuration. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this comment is superfluous as it describes what the if statement does. Mind removing it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, need to get over this habit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comments that describe what statements does are superfluous. Noted! Will remove them! |
||
|
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.
please don't add more than two newlines at a time :)
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.
Maybe worth adding? http://eslint.org/docs/rules/no-multiple-empty-lines
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.
+1 to no multiple empty lines in a separate PR!
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.
👍 for
no-multiple-empty-lines
like in StandardJS..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.
@abdulhannanali mind sending a separate PR that adds that lint rule? :)
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.
@cpojer Definitely thanks a lot! 👍