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

Only lint files that have changed in ci_lint.sh #6136

Closed
foolip opened this issue Jun 2, 2017 · 5 comments
Closed

Only lint files that have changed in ci_lint.sh #6136

foolip opened this issue Jun 2, 2017 · 5 comments

Comments

@foolip
Copy link
Member

foolip commented Jun 2, 2017

We are currently generating the whole manifest and linting all files in ci_lint.sh. This takes over 2 minutes on my desktop machine, and is probably much slower still on Travis.

Everything that needs to happen should be possible in <1 second for a typical PR:

  1. Generate a manifest only for the modified files. (Assuming that only a file itself affects the classification.)
  2. Run the lint only for modified files.

@bobholt linked to a discussion with @jgraham in http://logs.glob.uno/?c=w3%23testing&s=26+May+2017&e=26+May+2017

Do either of you know how long it currently takes on Travis in a typical case?

@jgraham
Copy link
Contributor

jgraham commented Jun 2, 2017

The way I discussed doing this with gsnedders is to cache the lint status with the file hash and not check files that haven't changed for lints that are based purely on the file contents. Travis can store cached artifacts.

I'm not sure what generating a manifest only for certain files would look like, but my way means you don't need to ask the VCS which files changed.

@foolip
Copy link
Member Author

foolip commented Jun 2, 2017

The manifest is used to determine what lint steps are run, right? I was thinking pass some command line arguments so that the manifest only includes a few files, those that are needed.

Or, the manifest itself could be cached, we've done something like that in Chromium to get around the long build times for it.

@jgraham
Copy link
Contributor

jgraham commented Jul 15, 2017

FWIW I have a branch that implements a simple version of this. Cuts the lint time pretty dramatically, but it depends on unmerged things in #6237

@jgraham
Copy link
Contributor

jgraham commented Jul 15, 2017

Erm, make that #6421

@bobholt
Copy link
Contributor

bobholt commented Oct 4, 2017

We merged this fix.

@bobholt bobholt closed this as completed Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants