Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2847723002:
Add files.watcherExlude to vscode.md (Closed)
DescriptionAdd files.watcherExlude to vscode.md
I had the issue that vscode didn't notice file changes e.g.
by git cl format. Excluding out/ and third_party/ with
files.watcherExclude seems to help.
Files per folder:
10934 v8
14436 native_client
17478 data
26470 build
37676 chrome
389528 third_party
426532 out
BUG=
Review-Url: https://codereview.chromium.org/2847723002
Cr-Commit-Position: refs/heads/master@{#467668}
Committed: https://chromium.googlesource.com/chromium/src/+/a347f0ebcb36d711a0ecb366b15815ce595f64bd
Patch Set 1 #
Total comments: 9
Dependent Patchsets: Messages
Total messages: 22 (8 generated)
dullweber@chromium.org changed reviewers: + chaopeng@chromium.org
Hi, vscode didn't notice external file changes and this setting seems to help. I guess it might be useful for others too.
lgtm
The CQ bit was checked by dullweber@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
dullweber@chromium.org changed reviewers: + ljusten@chromium.org
+ljusten as we are both no commiters yet
dullweber@chromium.org changed reviewers: + jdoerrie@chromium.org
+jdoerrie could you please lgtm this, no one here is a commiter :D
lgtm
The CQ bit was checked by dullweber@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Btw, you can TBR documentation changes, see https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#... https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md File docs/vscode.md (right): https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode194 docs/vscode.md:194: "files.watcherExclude": { Out of curiosity, did that cause any issues? Did you notice any improvement? https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode198 docs/vscode.md:198: "**/out/**": true, Suggest to use **/out*/** since IIRC some Chrome OS workflows generate out-$board directories. Is excluding out really necessary given that it's already excluded by files.exclude? https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode199 docs/vscode.md:199: "**/third_party/**": true The initial **/ seems unnecessary (files.excludes works without that), but according to jsturgis' comment on the github issue it appears to be required. Maybe add a comment?
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1493302200376000, "parent_rev":
"c186cf10dc3367044f9143308bc3452eff0cd22c", "commit_rev":
"a347f0ebcb36d711a0ecb366b15815ce595f64bd"}
Message was sent while issue was closed.
Description was changed from ========== Add files.watcherExlude to vscode.md I had the issue that vscode didn't notice file changes e.g. by git cl format. Excluding out/ and third_party/ with files.watcherExclude seems to help. Files per folder: 10934 v8 14436 native_client 17478 data 26470 build 37676 chrome 389528 third_party 426532 out BUG= ========== to ========== Add files.watcherExlude to vscode.md I had the issue that vscode didn't notice file changes e.g. by git cl format. Excluding out/ and third_party/ with files.watcherExclude seems to help. Files per folder: 10934 v8 14436 native_client 17478 data 26470 build 37676 chrome 389528 third_party 426532 out BUG= Review-Url: https://codereview.chromium.org/2847723002 Cr-Commit-Position: refs/heads/master@{#467668} Committed: https://chromium.googlesource.com/chromium/src/+/a347f0ebcb36d711a0ecb366b158... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/a347f0ebcb36d711a0ecb366b158...
Message was sent while issue was closed.
https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md File docs/vscode.md (right): https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode194 docs/vscode.md:194: "files.watcherExclude": { On 2017/04/27 14:16:24, ljusten wrote: > Out of curiosity, did that cause any issues? Did you notice any improvement? I had the problem that when I run git cl format, vscode didn't notice any changes to the currently open file. Adding watcherExclude fixed this. https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode198 docs/vscode.md:198: "**/out/**": true, On 2017/04/27 14:16:24, ljusten wrote: > Suggest to use **/out*/** since IIRC some Chrome OS workflows generate > out-$board directories. > > Is excluding out really necessary given that it's already excluded by > files.exclude? files.exlude is not applied to the watcher for some reason. Maybe the issue only hit me because I have quite a few different directories in out/ with different configurations. I guess it is mostly important to reduce the number of files that are being watched. Unless these chromeos folders also contains hundrets of thousands of files it probably doesn't matter. https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode199 docs/vscode.md:199: "**/third_party/**": true On 2017/04/27 14:16:24, ljusten wrote: > The initial **/ seems unnecessary (files.excludes works without that), but > according to jsturgis' comment on the github issue it appears to be required. > Maybe add a comment? It is currently required to have the initial **/, I guess that is a bug but it doesn't work without this. I subscribed to the github issue and will remove the **/ when it is fixed.
Message was sent while issue was closed.
On 2017/04/27 14:16:24, ljusten wrote: > Btw, you can TBR documentation changes, see > https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#... As far as I know it is not possible to TBR without a lgtm from a commiter if you are not a commiter.
Message was sent while issue was closed.
LGTM if you use **/out*/**. https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md File docs/vscode.md (right): https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode194 docs/vscode.md:194: "files.watcherExclude": { On 2017/04/27 14:27:29, dullweber wrote: > On 2017/04/27 14:16:24, ljusten wrote: > > Out of curiosity, did that cause any issues? Did you notice any improvement? > > I had the problem that when I run git cl format, vscode didn't notice any > changes to the currently open file. Adding watcherExclude fixed this. Acknowledged. https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode198 docs/vscode.md:198: "**/out/**": true, > I guess it is mostly important to reduce the number of files that are being > watched. Unless these chromeos folders also contains hundrets of thousands of > files it probably doesn't matter. The chromeos folders are regular build folders, so yes, they're big. https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode199 docs/vscode.md:199: "**/third_party/**": true On 2017/04/27 14:27:29, dullweber wrote: > On 2017/04/27 14:16:24, ljusten wrote: > > The initial **/ seems unnecessary (files.excludes works without that), but > > according to jsturgis' comment on the github issue it appears to be required. > > Maybe add a comment? > > It is currently required to have the initial **/, I guess that is a bug but it > doesn't work without this. I subscribed to the github issue and will remove the > **/ when it is fixed. Acknowledged.
Message was sent while issue was closed.
On 2017/04/27 15:54:42, ljusten wrote: > LGTM if you use **/out*/**. > > https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md > File docs/vscode.md (right): > > https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode194 > docs/vscode.md:194: "files.watcherExclude": { > On 2017/04/27 14:27:29, dullweber wrote: > > On 2017/04/27 14:16:24, ljusten wrote: > > > Out of curiosity, did that cause any issues? Did you notice any improvement? > > > > I had the problem that when I run git cl format, vscode didn't notice any > > changes to the currently open file. Adding watcherExclude fixed this. > > Acknowledged. > > https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode198 > docs/vscode.md:198: "**/out/**": true, > > I guess it is mostly important to reduce the number of files that are being > > watched. Unless these chromeos folders also contains hundrets of thousands of > > files it probably doesn't matter. > > The chromeos folders are regular build folders, so yes, they're big. > > https://codereview.chromium.org/2847723002/diff/1/docs/vscode.md#newcode199 > docs/vscode.md:199: "**/third_party/**": true > On 2017/04/27 14:27:29, dullweber wrote: > > On 2017/04/27 14:16:24, ljusten wrote: > > > The initial **/ seems unnecessary (files.excludes works without that), but > > > according to jsturgis' comment on the github issue it appears to be > required. > > > Maybe add a comment? > > > > It is currently required to have the initial **/, I guess that is a bug but it > > doesn't work without this. I subscribed to the github issue and will remove > the > > **/ when it is fixed. > > Acknowledged. The cl was already submitted so I fixed the pattern in another cl: http://crrev.com/2847663004 Until the leading **/ is not required anymore this will also match src/cc/output/, I added a comment to warn about this. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
