Issue 22542003: analyzer_exp fixes

Issue 22542003: analyzer_exp fixes (Closed)

Created:
7 years, 4 months ago by kevmoo-old

Modified:
7 years, 4 months ago

Reviewers:
scheglov, pquitslund

CC:
reviews_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

analyzer_exp fixes R=scheglov@google.com Committed: https://code.google.com/p/dart/source/detail?r=25946

Patch Set 1 #

Total comments: 9

Created: 7 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+22 lines, -51 lines)			Patch
M	pkg/analyzer_experimental/bin/analyzer.dart	View	1 chunk	+0 lines, -6 lines	0 comments	Download
M	pkg/analyzer_experimental/lib/src/analyzer_impl.dart	View	2 chunks	+0 lines, -2 lines	0 comments	Download
M	pkg/analyzer_experimental/lib/src/error.dart	View	1 chunk	+0 lines, -1 line	0 comments	Download
M	pkg/analyzer_experimental/lib/src/error_formatter.dart	View	1 chunk	+0 lines, -8 lines	0 comments	Download
M	pkg/analyzer_experimental/lib/src/services/formatter_impl.dart	View	4 chunks	+7 lines, -7 lines	4 comments	Download
M	pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart	View	2 chunks	+0 lines, -3 lines	0 comments	Download
M	pkg/analyzer_experimental/lib/src/services/writer.dart	View	5 chunks	+15 lines, -22 lines	5 comments	Download
M	pkg/analyzer_experimental/test/services/test_utils.dart	View	1 chunk	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

kevmoo-old

Committed patchset #1 manually as r25946 (presubmit successful).

7 years, 4 months ago (2013-08-08 21:07:09 UTC) #3

pquitslund

Thanks for the input! I'm not sure I completely agree with some of the more ...

7 years, 4 months ago (2013-08-08 21:20:23 UTC) #4

kevmoo-old

https://codereview.chromium.org/22542003/diff/1/pkg/analyzer_experimental/lib/src/services/formatter_impl.dart File pkg/analyzer_experimental/lib/src/services/formatter_impl.dart (right): https://codereview.chromium.org/22542003/diff/1/pkg/analyzer_experimental/lib/src/services/formatter_impl.dart#newcode43 pkg/analyzer_experimental/lib/src/services/formatter_impl.dart:43: final String message; On 2013/08/08 21:20:23, pquitslund wrote: > ...

7 years, 4 months ago (2013-08-08 21:23:34 UTC) #5

pquitslund

7 years, 4 months ago (2013-08-08 21:37:55 UTC) #6

Message was sent while issue was closed.

On 2013/08/08 21:23:34, kevmoo wrote:
>
https://codereview.chromium.org/22542003/diff/1/pkg/analyzer_experimental/lib...
> File pkg/analyzer_experimental/lib/src/services/formatter_impl.dart (right):
> 
>
https://codereview.chromium.org/22542003/diff/1/pkg/analyzer_experimental/lib...
> pkg/analyzer_experimental/lib/src/services/formatter_impl.dart:43: final
String
> message;
> On 2013/08/08 21:20:23, pquitslund wrote:
> > Hmmmm.  The type seems gratuitous here.  Or?
> 
> Nope. It's great for the analyzer and doc generation. Inline, dynamic is
great,
> but for public API, default to type info.

But this isn't public API!

> 
>
https://codereview.chromium.org/22542003/diff/1/pkg/analyzer_experimental/lib...
> pkg/analyzer_experimental/lib/src/services/formatter_impl.dart:121: void
> checkForErrors() {
> On 2013/08/08 21:20:23, pquitslund wrote:
> > What's your thinking on when to specify a void return type?  I'm still
feeling
> > it out but I sort of prefer to leave it off where it isn't confusing.
> 
> If a public member returns nothing, flag it void.

It's only public within the library but this library is intentionally NOT
exported.

> 
> Otherwise, one is left wondering if something is returned...what it's used
for,
> etc.

Ok, so I agree here completely assuming this were API.  That part isn't settled
yet though.

Expand Messages | Collapse Messages