Errors: Support a filter to control which errors are reported #728

mrjoelkemp · 2014-10-30T03:58:57Z

Fixes #335

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-10-30T04:02:11Z

Coverage decreased (-0.06%) when pulling 5192449 on mrjoelkemp:error_filter into 101eccb on jscs-dev:master.

mrjoelkemp · 2014-10-30T04:03:59Z

README.md

+
+  // Return false to exclude this error
+  // Return true to report this error normally
+  return false;


The logic behind false was that it was like disallowing the propagation of the error to the reporter.

If enough people think true should throw the current error away, then I'll change it.

This is fine and a good description, but perhaps "writing a filter and writing a reporter" belong elsewhere in the docs.

This is a good point; the readme is pretty cluttered as is. I'm going to create a wiki page and link to it.

mikesherov · 2014-10-30T11:02:23Z

@mrjoelkemp, nice work! Need a CLI option too, right?

mdevils · 2014-10-30T20:43:55Z

Nice work! CLI option would be nice.

BTW Let's not merge before #731 please.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-10-31T01:04:58Z

Coverage decreased (-0.05%) when pulling 55add29 on mrjoelkemp:error_filter into 101eccb on jscs-dev:master.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-10-31T01:14:42Z

Coverage decreased (-0.06%) when pulling aa766f8 on mrjoelkemp:error_filter into 101eccb on jscs-dev:master.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-07T04:21:15Z

Coverage decreased (-0.38%) when pulling 3bb0b72 on mrjoelkemp:error_filter into 52dcc50 on jscs-dev:master.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-07T04:29:18Z

Coverage decreased (-0.01%) when pulling 4817187 on mrjoelkemp:error_filter into 52dcc50 on jscs-dev:master.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-07T04:33:29Z

Coverage decreased (-0.01%) when pulling 2908a91 on mrjoelkemp:error_filter into 52dcc50 on jscs-dev:master.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-09T23:02:34Z

Coverage decreased (-0.01%) when pulling 11260f5 on mrjoelkemp:error_filter into 573ef5a on jscs-dev:master.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-09T23:11:22Z

Coverage increased (+0.03%) when pulling 7d6dc3d on mrjoelkemp:error_filter into 573ef5a on jscs-dev:master.

mdevils · 2014-11-10T00:22:02Z

lib/config/configuration.js

+            typeof config.errorFilter === 'string' || config.errorFilter === null,
+            '`errorFilter` option requires a string or null value'
+        );
+        this._errorFilter = require(config.errorFilter.replace('.js', ''));


This will fail for browser-version. You should declare a method like _loadPlugin and override it in NodeConfiguration.

We probably need to add some smoke tests for browser version, like with phantom, not sure if would need something more besides that

Yeah, we should!

Created - #757

@mdevils This is a great catch. This implies that an errorFilter is a function in lib/config/configuration and a string in lib/config/node-configuration.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-11T05:23:59Z

Coverage increased (+0.04%) when pulling 7556fdc on mrjoelkemp:error_filter into 573ef5a on jscs-dev:master.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-11T05:56:26Z

Coverage increased (+0.04%) when pulling 3957b9d on mrjoelkemp:error_filter into 573ef5a on jscs-dev:master.

mrjoelkemp · 2014-11-11T05:59:28Z

lib/config/configuration.js

@@ -258,6 +270,10 @@ Configuration.prototype._processConfig = function(config) {
        this._esnextEnabled = Boolean(config.esnext);
    }

+    if (config.errorFilter) {
+        this._loadErrorFilter(config.errorFilter);


@mdevils The fact that configuration jumps between its own functions and the overridden functions of node-configuration made this hard to follow and implement. I understand why this has to happen, but do you feel it would be more unclear if we only have lib/config/configuration and use try/catch to determine if we're in the node or browser environment?

if we only have lib/config/configuration and use try/catch to determine if we're in the node or browser environment?

I think this will make the code more complext and lead to the future breaks, because you will have to include some external modules with hacks (to disallow browserify include it), put many catches. So currently I think we have simpler solution.

That's fair. Thanks for the insight.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-11T06:04:37Z

Coverage increased (+0.04%) when pulling 34172b4 on mrjoelkemp:error_filter into 573ef5a on jscs-dev:master.

mrjoelkemp · 2014-11-11T13:29:23Z

Ready for re-review: @mdevils and @markelog

mrjoelkemp · 2014-11-15T03:41:39Z

@mikesherov Please give this a final look over when you have a chance. Just need a thumbs up to merge this. Thanks in advance.

mikesherov · 2014-11-15T13:48:50Z

lib/errors.js

+     *
+     * @param {Function} filter
+     */
+    filterErrorList: function(filter) {


Seems like this function should simply be called filter. errors.filterErrorList is redundant.

Closes jscs-devgh-728 Fixes jscs-dev#335

coveralls · 2014-11-15T16:18:37Z

Coverage increased (+0.03%) when pulling 2ca671b on mrjoelkemp:error_filter into b54645c on jscs-dev:master.

mikesherov · 2014-11-15T19:41:52Z

👍 LGTM to land.

mrjoelkemp · 2014-11-15T21:03:29Z

Yay! Thanks for reviewing, all.

mikesherov · 2014-11-15T21:40:07Z

Great work Joel!

On Saturday, November 15, 2014, Joel Kemp notifications@github.com wrote:

Yay! Thanks for reviewing, all.

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

Mike Sherov

mrjoelkemp force-pushed the error_filter branch from 6968a3e to 5192449 Compare October 30, 2014 03:59

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Oct 30, 2014

Errors: Support a filter to control which errors are reported

5192449

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp added enhancement new option error reporting labels Oct 30, 2014

mrjoelkemp reviewed Oct 30, 2014
View reviewed changes

mrjoelkemp force-pushed the error_filter branch from 5192449 to 73e9560 Compare October 31, 2014 00:59

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Oct 31, 2014

Errors: Support a filter to control which errors are reported

73e9560

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from 73e9560 to 55add29 Compare October 31, 2014 01:00

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Oct 31, 2014

Errors: Support a filter to control which errors are reported

55add29

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from 55add29 to aa766f8 Compare October 31, 2014 01:11

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Oct 31, 2014

Errors: Support a filter to control which errors are reported

aa766f8

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from aa766f8 to 3bb0b72 Compare November 7, 2014 04:18

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 7, 2014

Errors: Support a filter to control which errors are reported

3bb0b72

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from 3bb0b72 to 4817187 Compare November 7, 2014 04:26

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 7, 2014

Errors: Support a filter to control which errors are reported

4817187

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from 4817187 to 2908a91 Compare November 7, 2014 04:30

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 7, 2014

Errors: Support a filter to control which errors are reported

2908a91

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 9, 2014

Errors: Support a filter to control which errors are reported

11260f5

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from 11260f5 to 7d6dc3d Compare November 9, 2014 23:09

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 9, 2014

Errors: Support a filter to control which errors are reported

7d6dc3d

Closes jscs-devgh-728 Fixes jscs-dev#335

mdevils reviewed Nov 10, 2014
View reviewed changes

mrjoelkemp force-pushed the error_filter branch from 7d6dc3d to 7556fdc Compare November 11, 2014 05:21

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 11, 2014

Errors: Support a filter to control which errors are reported

7556fdc

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from 7556fdc to 3957b9d Compare November 11, 2014 05:53

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 11, 2014

Errors: Support a filter to control which errors are reported

3957b9d

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp reviewed Nov 11, 2014
View reviewed changes

mrjoelkemp force-pushed the error_filter branch from 3957b9d to 34172b4 Compare November 11, 2014 06:01

mrjoelkemp pushed a commit to mrjoelkemp/node-jscs that referenced this pull request Nov 11, 2014

Errors: Support a filter to control which errors are reported

34172b4

Closes jscs-devgh-728 Fixes jscs-dev#335

mikesherov reviewed Nov 15, 2014
View reviewed changes

Errors: Support a filter to control which errors are reported

2ca671b

Closes jscs-devgh-728 Fixes jscs-dev#335

mrjoelkemp force-pushed the error_filter branch from 34172b4 to 2ca671b Compare November 15, 2014 16:14

mrjoelkemp merged commit 2ca671b into jscs-dev:master Nov 15, 2014

mrjoelkemp deleted the error_filter branch November 15, 2014 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Errors: Support a filter to control which errors are reported #728

Errors: Support a filter to control which errors are reported #728

mrjoelkemp commented Oct 30, 2014

coveralls commented Oct 30, 2014

mrjoelkemp Oct 30, 2014

mikesherov Nov 8, 2014

mrjoelkemp Nov 8, 2014

mikesherov commented Oct 30, 2014

mdevils commented Oct 30, 2014

coveralls commented Oct 31, 2014

coveralls commented Oct 31, 2014

coveralls commented Nov 7, 2014

coveralls commented Nov 7, 2014

coveralls commented Nov 7, 2014

coveralls commented Nov 9, 2014

coveralls commented Nov 9, 2014

mdevils Nov 10, 2014

markelog Nov 10, 2014

mdevils Nov 10, 2014

markelog Nov 10, 2014

mrjoelkemp Nov 11, 2014

coveralls commented Nov 11, 2014

coveralls commented Nov 11, 2014

mrjoelkemp Nov 11, 2014

mdevils Nov 11, 2014

mrjoelkemp Nov 11, 2014

coveralls commented Nov 11, 2014

mrjoelkemp commented Nov 11, 2014

mrjoelkemp commented Nov 15, 2014

mikesherov Nov 15, 2014

coveralls commented Nov 15, 2014

mikesherov commented Nov 15, 2014

mrjoelkemp commented Nov 15, 2014

mikesherov commented Nov 15, 2014

Errors: Support a filter to control which errors are reported #728

Errors: Support a filter to control which errors are reported #728

Conversation

mrjoelkemp commented Oct 30, 2014

coveralls commented Oct 30, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikesherov commented Oct 30, 2014

mdevils commented Oct 30, 2014

coveralls commented Oct 31, 2014

coveralls commented Oct 31, 2014

coveralls commented Nov 7, 2014

coveralls commented Nov 7, 2014

coveralls commented Nov 7, 2014

coveralls commented Nov 9, 2014

coveralls commented Nov 9, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 11, 2014

coveralls commented Nov 11, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 11, 2014

mrjoelkemp commented Nov 11, 2014

mrjoelkemp commented Nov 15, 2014

Choose a reason for hiding this comment

coveralls commented Nov 15, 2014

mikesherov commented Nov 15, 2014

mrjoelkemp commented Nov 15, 2014

mikesherov commented Nov 15, 2014