Skip to content
This repository was archived by the owner on Oct 23, 2024. It is now read-only.

Annotate the type of 'variables' in class All and class None #5

Merged
merged 2 commits into from
Mar 6, 2018

Conversation

a-siva
Copy link
Contributor

@a-siva a-siva commented Mar 6, 2018

Annotate the type of 'variables' in class All and class None to ensure that we do not run into dart-lang/sdk#32412

@a-siva a-siva requested a review from nex3 March 6, 2018 15:46
lib/src/all.dart Outdated
@@ -6,7 +6,7 @@ import '../boolean_selector.dart';

/// A selector that matches all inputs.
class All implements BooleanSelector {
final variables = const [];
final List<String> variables = const [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this go on the right -> <String>[]?

pubspec.yaml Outdated
@@ -1,5 +1,5 @@
name: boolean_selector
version: 1.0.3-dev
version: 1.0.4-dev
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think this is needed, since 1.0.3 has not been released

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this should just be 1.0.3.

Copy link
Contributor

@nex3 nex3 left a comment

Choose a reason for hiding this comment

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

Please add comments to the variables indicating why they're over-specified and linking to the tracking SDK issue.

CHANGELOG.md Outdated
@@ -1,3 +1,7 @@
## 1.0.3

* Annotate the type of 'variables' in class All and class None.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd write "Work around a common front-end inference bug." The changelog is intended for downstream users, and as far as they're concerned the only change here is that this now works with the CFE.

pubspec.yaml Outdated
@@ -1,5 +1,5 @@
name: boolean_selector
version: 1.0.3-dev
version: 1.0.4-dev
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this should just be 1.0.3.

Copy link
Contributor

@nex3 nex3 left a comment

Choose a reason for hiding this comment

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

Made changes myself since @a-siva is OOO today.

@nex3 nex3 merged commit 83b690b into dart-archive:master Mar 6, 2018
@a-siva
Copy link
Contributor Author

a-siva commented Mar 7, 2018

Thanks

@a-siva a-siva deleted the fix32412 branch March 7, 2018 14:15
mosuem pushed a commit to dart-lang/tools that referenced this pull request Oct 18, 2024
Annotate the type of All.variables and None.variables to work around
dart-lang/sdkdart-lang/boolean_selector#32412.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants