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

perf(reporter): add option to limit result types to be processed #568

Merged
merged 2 commits into from
Oct 18, 2017

Conversation

isner
Copy link
Contributor

@isner isner commented Oct 10, 2017

This allows us to specify a short list of result types that we want helper.processAggregate to bother processing. For some reporting scenarios, we don't want to bother processing 'passes', for instance. This causes a massive performance improvement for those scenarios.

We can leverage this option in the axe extension to speed up performance there as well.

Closes #512

Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Please add some tests and document this feature.

@isner
Copy link
Contributor Author

isner commented Oct 13, 2017

@WilcoFiers Tests added. Let me know if the documentation is insufficient.

@@ -231,7 +231,7 @@ axe.run(context, options, callback);
#### Parameters axe.run

* [`context`](#context-parameter): (optional) Defines the scope of the analysis - the part of the DOM that you would like to analyze. This will typically be the `document` or a specific selector such as class name, ID, selector, etc.
* [`options`](#options-parameter): (optional) Set of options passed into rules or checks, temporarily modifying them. This contrasts with `axe.configure`, which is more permanent. [See below for more information](#axerun-parameters)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The documentation for axe.configure is above, not below, and the internal link is invalid. I figured we ought to simply remove this.

@WilcoFiers WilcoFiers merged commit 42b46d9 into develop Oct 18, 2017
@dylanb
Copy link
Contributor

dylanb commented Nov 6, 2017

@WilcoFiers @isner did the CircleCI build run for this PR?

@marcysutton
Copy link
Contributor

@dylanb it says it did, you can find the integrations under "View details" on the merged commit above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants