Subject: [quicwg/base-drafts] Make lint faster (#938)

From: Mike Bishop <[email-masked]>
Date: Thu, 16 Nov 2017 10:22:35 +0000 (UTC)

Ian and I were remarking that makes and commits have gotten very slow lately.  Some observation of my machine shows that I'm spending twelve seconds checking for trailing whitespace and long lines.

This updates the `make lint` recipe to cache the hashes per branch of files that have successfully been linted and not re-lint them unless they've been modified.  On my machine, lint goes from 12 seconds to 0.75 seconds.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/938

-- Commit Summary --

  * Make lint faster

-- File Changes --

    M .gitignore (1)
    M Makefile (39)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/938.patch
https://github.com/quicwg/base-drafts/pull/938.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938

---

From: Mike Bishop <[email-masked]>
Date: Fri, 17 Nov 2017 00:38:00 +0000 (UTC)

You can make it do the same for new branches too, but it requires a git `post-checkout` hook, which is a local configuration (unless we get the i-d-template to bring the hook along like it does the `pre-commit` hook).  Here's the hook script:

```
#!/bin/bash

new_lintcache=".lintcache/`git rev-parse --abbrev-ref HEAD`";

if [ ! -d "$new_lintcache" ]; then
    for branch in `git branch --points-at $2`; do
        old_lintcache=".lintcache/$branch";
        if [ "$old_lintcache" != "$new_lintcache" ] && [ -d "$old_lintcache" ]; then
            cp -r $old_lintcache $new_lintcache;
            break;
        fi
    done
fi
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#issuecomment-345109134

---

From: Martin Thomson <[email-masked]>
Date: Fri, 17 Nov 2017 00:46:03 +0000 (UTC)

martinthomson requested changes on this pull request.

I think that there is a simpler design.

Have lint depend on `.lintcache/filename` and generate that file as a result of linting `filename`.  Let make do all the heavy lifting.

The downside of that approach is that make will stop after the first error.  We can fix that on CI at least by using `make -k` on an explicit linting step.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#pullrequestreview-77311488

---

From: janaiyengar <[email-masked]>
Date: Fri, 17 Nov 2017 00:56:50 +0000 (UTC)

(sorry, nothing useful to offer at the end of a long IETF week, but I really want to rename this "Make lint fast again").

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#issuecomment-345112274

---

From: Mike Bishop <[email-masked]>
Date: Fri, 17 Nov 2017 01:01:02 +0000 (UTC)

There you go, @janaiyengar.  ;-)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#issuecomment-345112989

---

From: Mike Bishop <[email-masked]>
Date: Fri, 17 Nov 2017 01:00:31 +0000 (UTC)

You're right, that's a better design.  Also, I can fold the branch migration into that -- check at lint time whether I can steal a lint file from another branch rather than actually running the lint commands.

I'll rearchitect that later.  We should also perhaps update the Circle config to cache .lintcache as well.  Does that mean I need to make the directory controllable by an env variable?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#issuecomment-345112891

---

From: Martin Thomson <[email-masked]>
Date: Fri, 17 Nov 2017 02:01:24 +0000 (UTC)

Caching .lintcache won't work because the code is checked out from scratch every time, so modification times are updated.  Paying the price on CI isn't that bad for the moment.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#issuecomment-345122858

---

From: Mike Bishop <[email-masked]>
Date: Fri, 17 Nov 2017 07:37:07 +0000 (UTC)

@MikeBishop pushed 1 commit.

e5a0bee  Use Makefile to manage lint cache

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/938/files/f4c5a0e7908fc862f13b54839035e9a5eff787ef..e5a0bee163bc71dbdd46b04d22bc0da5b025d91f

---

From: Mike Bishop <[email-masked]>
Date: Fri, 17 Nov 2017 07:39:13 +0000 (UTC)

I think it should, actually, because the content of the .lint_cache file is the hash of the Markdown file.  If the file hasn't changed in the latest commit, all you pay is the hashing cost to double-check that.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#issuecomment-345168382

---

From: Mike Bishop <[email-masked]>
Date: Fri, 17 Nov 2017 07:48:52 +0000 (UTC)

be3f5e3  Avoid warnings from cp

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/938/files/e5a0bee163bc71dbdd46b04d22bc0da5b025d91f..be3f5e3fccc9f32b9cc6b923397fc1dd6e502e2c

---

From: Martin Thomson <[email-masked]>
Date: Mon, 27 Nov 2017 02:58:02 +0000 (UTC)

I don't think that optimizing for re-runs in CI is worthwhile.  And maybe you misunderstood my suggestion.  This is much more complicated than I think we need.  I'll show you what I mean with another PR.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/938#issuecomment-347066486