Skip to content

Conversation

@matanlurey
Copy link
Contributor

Closes #173

  • Reading a field that doesn't exist now throws FormatException

Closes #169

  • Now supports listValue and mapValue

@matanlurey matanlurey requested review from jakemac53 and kevmoo June 22, 2017 00:32
@matanlurey matanlurey added this to the v0.5.9 milestone Jun 22, 2017
@kevmoo
Copy link
Member

kevmoo commented Jun 22, 2017

@matanlurey appease the travis gods...

Copy link
Member

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question

dependencies:
analyzer: '>=0.29.2 <0.31.0'
build: ^0.9.0
collection: ^1.1.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this an oops from a previous commit?

var element = root;
while (element != null) {
final field = element.getField(name);
if (field != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] consider collapsing this to 1 line so it doesn't take more visual weight than it earns.

element = element.supertype?.element;
}
final allFields = root.fields.toSet();
root.allSupertypes.forEach((t) => allFields.addAll(t.element.fields));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] https://www.dartlang.org/guides/language/effective-dart/usage#avoid-using-iterableforeach-with-a-function-literal

Could use a for loop, or I think it might still fit on 1 line to split this into 2 steps.

root.allSupertype.map((t) => t.element.fields).forEach(allFields.addAll);

or

allFields.addAll(root.allSupertypes.expand((t) => t.element.fields));

/// Throws [FormatException] if [isInt] is `false`.
int get intValue;

/// Returns whether this constant represents a `List` literal.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] get methods should be documented like a variable or a field - with a noun so you can omit 'Returns'.

Similar below.

@matanlurey matanlurey merged commit 60a4b38 into dart-lang:master Jun 22, 2017
@matanlurey matanlurey deleted the constant-more branch June 22, 2017 04:32
@matanlurey
Copy link
Contributor Author

@natebosch Oops. I just saw this after refreshing. I'll fix comments upstream.

mosuem pushed a commit to dart-lang/build that referenced this pull request Dec 10, 2024
* Add a Constant helper class.

* Add List and Map.

* Add missing field handling.

* Oops.

* Fix tests.
mosuem pushed a commit to dart-lang/build that referenced this pull request Dec 10, 2024
* Add a Constant helper class.

* Add List and Map.

* Add missing field handling.

* Oops.

* Fix tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants