Issue 10823257: Refactored accessors in mirrors from methods to properties.

Issue 10823257: Refactored accessors in mirrors from methods to properties. (Closed)

Created:
8 years, 4 months ago by Johnni Winther

Modified:
8 years, 4 months ago

Reviewers:
turnidge

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Refactored accessors in mirrors from methods to properties. Committed: https://code.google.com/p/dart/source/detail?r=10544

Patch Set 2 : Updated cf. comments #

Patch Set 3 : Missing updates #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+457 lines, -467 lines)			Patch
M	lib/dartdoc/comment_map.dart	View		3 chunks	+6 lines, -6 lines	0 comments	Download
M	lib/dartdoc/dartdoc.dart	View		40 chunks	+100 lines, -100 lines	0 comments	Download
M	lib/dartdoc/mirrors/dart2js_mirror.dart	View	1 2	46 chunks	+126 lines, -129 lines	0 comments	Download
M	lib/dartdoc/mirrors/mirrors.dart	View		17 chunks	+39 lines, -46 lines	0 comments	Download
M	lib/dartdoc/mirrors/mirrors_util.dart	View	1 2	4 chunks	+11 lines, -11 lines	0 comments	Download
M	lib/dartdoc/utils.dart	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	tests/compiler/dart2js/mirrors_test.dart	View		23 chunks	+119 lines, -119 lines	0 comments	Download
M	utils/apidoc/apidoc.dart	View		11 chunks	+22 lines, -22 lines	0 comments	Download
M	utils/apidoc/html_diff.dart	View	1 2	9 chunks	+18 lines, -18 lines	0 comments	Download
M	utils/apidoc/html_diff_dump.dart	View	1 2	3 chunks	+14 lines, -14 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Johnni Winther

mirrors.dart is the point of interest. The other files are just updated accordingly.

8 years, 4 months ago (2012-08-09 16:34:05 UTC) #1

turnidge

lgtm http://codereview.chromium.org/10823257/diff/1/lib/dartdoc/mirrors/dart2js_mirror.dart File lib/dartdoc/mirrors/dart2js_mirror.dart (right): http://codereview.chromium.org/10823257/diff/1/lib/dartdoc/mirrors/dart2js_mirror.dart#newcode615 lib/dartdoc/mirrors/dart2js_mirror.dart:615: bool get isOptional() => _isOptional; This could be ...

8 years, 4 months ago (2012-08-09 17:44:13 UTC) #2

Johnni Winther

8 years, 4 months ago (2012-08-10 23:40:23 UTC) #3

http://codereview.chromium.org/10823257/diff/1/lib/dartdoc/mirrors/dart2js_mi...
File lib/dartdoc/mirrors/dart2js_mirror.dart (right):

http://codereview.chromium.org/10823257/diff/1/lib/dartdoc/mirrors/dart2js_mi...
lib/dartdoc/mirrors/dart2js_mirror.dart:615: bool get isOptional() =>
_isOptional;
On 2012/08/09 17:44:13, turnidge wrote:
> This could be made a final bool field instead of a getter.  Your call.

Done.

http://codereview.chromium.org/10823257/diff/1/lib/dartdoc/mirrors/dart2js_mi...
lib/dartdoc/mirrors/dart2js_mirror.dart:706: LibraryMirror get library() {
On 2012/08/09 17:44:13, turnidge wrote:
> Ditto here.  You get the idea.  Again, your call.

Done.

http://codereview.chromium.org/10823257/diff/1/lib/dartdoc/mirrors/mirrors.dart
File lib/dartdoc/mirrors/mirrors.dart (right):

http://codereview.chromium.org/10823257/diff/1/lib/dartdoc/mirrors/mirrors.da...
lib/dartdoc/mirrors/mirrors.dart:93: final Map<Object, MemberMirror>
declaredMembers;
On 2012/08/09 17:44:13, turnidge wrote:
> In my implementation, I just call this "members".  What do you think?

Using a 'lookup' method to provide access to inherited members will enable us to
use the shorter name 'members', so let's do that.

Expand Messages | Collapse Messages