Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1414133002:
document analysis_options (Closed)
|
Created:
5 years, 2 months ago by sethladd Modified:
5 years, 2 months ago CC:
reviews_dartlang.org Base URL:
git@github.com:dart-lang/sdk.git@master Target Ref:
refs/heads/master Visibility:
Public. |
Descriptiondocument analysis_options
BUG=
R=brianwilkerson@google.com, johnmccutchan@google.com, pquitslund@google.com
Committed: https://github.com/dart-lang/sdk/commit/527c163becbfe34b8c545f60cb67efb6ff30ab3e
Patch Set 1 #
Total comments: 7
Patch Set 2 : link to strong mode #
Total comments: 12
Patch Set 3 : tweaks #Messages
Total messages: 10 (2 generated)
sethladd@google.com changed reviewers: + johnmccutchan@google.com, pquitslund@google.com
brianwilkerson@google.com changed reviewers: + brianwilkerson@google.com
LGTM https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md File pkg/analyzer/README.md (right): https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... pkg/analyzer/README.md:20: and paths are analyzed, which lints are applied, and more. Last I checked (a couple of days ago), lints were not completely connected end-to-end. I would remove discussion of them until they're complete. https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... pkg/analyzer/README.md:24: and configuring the analyzer. There is at least one utility class that can help: AnalysisOptionsProvider. It can read and parse the file once it's been located. https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... pkg/analyzer/README.md:49: Here is an example file that enables two lint rules: Again, I'd remove this (lines 49-70) for now.
LGTM with comments https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md File pkg/analyzer/README.md (right): https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:27: at the root of your project (typically, next to your `pubspec.yaml`). remove 'typically' https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:39: - test/_data/p5/p5.dart Show an example of globbing: - 'test/_data/bad*.dart' Show an example of recursive globbing: - 'test/_brokendata/**' Call out that the glob syntax is defined by the glob package.
LGTM! https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md File pkg/analyzer/README.md (right): https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:14: by emailing our [list][]. Is this shorthand for [list][list]? https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:27: at the root of your project (typically, next to your `pubspec.yaml`). On 2015/10/20 01:10:36, Cutch wrote: > remove 'typically' Just another motivation to formalize (though I'm not sure where) our notion of a 'project'. Seth: is your intent to allow for the case where folks are using a `.packages` file and no pubspec? https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:39: - test/_data/p5/p5.dart On 2015/10/20 01:10:36, Cutch wrote: > Show an example of globbing: > > - 'test/_data/bad*.dart' > > Show an example of recursive globbing: > - 'test/_brokendata/**' > > Call out that the glob syntax is defined by the glob package. +1. FWIW: this is enough of a potential snag that I'm going to add validation and error-reporting to catch this case. https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:90: The API's in this package are, quite frankly, a mess at the moment. They were API's => APIs ? https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:96: API's. We plan to address this issue but doing so will, unfortunately, API's => APIs ?
On 2015/10/20 00:16:41, Brian Wilkerson wrote: > LGTM > > https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md > File pkg/analyzer/README.md (right): > > https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... > pkg/analyzer/README.md:20: and paths are analyzed, which lints are applied, and > more. > Last I checked (a couple of days ago), lints were not completely connected > end-to-end. I would remove discussion of them until they're complete. Right: not in 1.13 but slated for sure in 1.14 and we're very close in HEAD. Your call but since this is bleeding-edge I think we can live dangerously. > > https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... > pkg/analyzer/README.md:24: and configuring the analyzer. > There is at least one utility class that can help: AnalysisOptionsProvider. It > can read and parse the file once it's been located. > > https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... > pkg/analyzer/README.md:49: Here is an example file that enables two lint rules: > Again, I'd remove this (lines 49-70) for now.
https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md File pkg/analyzer/README.md (right): https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... pkg/analyzer/README.md:20: and paths are analyzed, which lints are applied, and more. Ah, interesting. How long until they are completed connected? https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... pkg/analyzer/README.md:24: and configuring the analyzer. Where is that file? in this analyzer package? https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md File pkg/analyzer/README.md (right): https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:14: by emailing our [list][]. yup! https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:27: at the root of your project (typically, next to your `pubspec.yaml`). On 2015/10/20 01:10:36, Cutch wrote: > remove 'typically' Done. https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:39: - test/_data/p5/p5.dart On 2015/10/20 01:10:36, Cutch wrote: > Show an example of globbing: > > - 'test/_data/bad*.dart' > > Show an example of recursive globbing: > - 'test/_brokendata/**' > > Call out that the glob syntax is defined by the glob package. Done. https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:90: The API's in this package are, quite frankly, a mess at the moment. They were On 2015/10/20 15:48:01, pquitslund wrote: > API's => APIs ? Done. https://codereview.chromium.org/1414133002/diff/20001/pkg/analyzer/README.md#... pkg/analyzer/README.md:96: API's. We plan to address this issue but doing so will, unfortunately, On 2015/10/20 15:48:01, pquitslund wrote: > API's => APIs ? Done.
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 527c163becbfe34b8c545f60cb67efb6ff30ab3e (presubmit successful).
Message was sent while issue was closed.
https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md File pkg/analyzer/README.md (right): https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... pkg/analyzer/README.md:20: and paths are analyzed, which lints are applied, and more. Currently targeted for 1.14. https://codereview.chromium.org/1414133002/diff/1/pkg/analyzer/README.md#newc... pkg/analyzer/README.md:24: and configuring the analyzer. On 2015/10/20 18:32:14, sethladd wrote: > Where is that file? in this analyzer package? pkg/analyzer/lib/source/analysis_options_provider.dart |
