-
Notifications
You must be signed in to change notification settings - Fork 94
Heuristically filter out minified files #114
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
Conversation
By default we filter out minified files that are identifiable as minified from their filename (e.g. *.min.js), but many projects have minified files that don't match those patterns, and these files can cause ESLint to segfault in a container environment (possibly due to memory pressure). This adds a new heuristic filter based on line length that filters out files that appear to be minified to prevent them being analyzed. For any matching files, a log message is sent to stderr notifying that the file is being skipped.
BatchSanitizer.prototype.sanitizedFiles = function() { | ||
var sanitizedFiles = []; | ||
|
||
for(var i = 0; i < this.files.length; i++) { |
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.
Any reason you did this with a for loop and not something like forEach
?
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.
no reason. Want me to switch to forEach
instead?
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.
Meh - don't feel strongly. This is fine!
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.
This was because I honestly forgot Node had forEach
.
Which repos? Were any of them particularly large (lots of files, large files, etc)? |
@gordondiggs tested on a variety of sizes - shortest run took 4 seconds, longest 16 minutes (mocha). Public repos from sample: |
@ABaldwinHunter We've used |
Code LGTM - curious how it affects relax |
@gordondiggs Thanks! Relax looks good |
By default we filter out minified files that are identifiable as minified from
their filename (e.g. *.min.js), but many projects have minified files that
don't match those patterns, and these files can cause ESLint to segfault in a
container environment (possibly due to memory pressure).
This adds a new heuristic filter based on line length that filters out files
that appear to be minified to prevent them being analyzed. For any matching
files, a log message is sent to stderr notifying that the file is being
skipped.
@codeclimate/review @wfleming
Note: tested locally on 8 repos and saw minimal performance loss. There are some minified files that we currently analyze successfully and this change would mean ignoring them instead. We think that is okay.