Skip to content

Fix parse errors with --preview-dart-2 #1874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/src/http.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ class _PubHttpClient extends http.BaseClient {
var streamedResponse;
try {
streamedResponse = await _inner.send(request);
} on SocketException catch (error, stackTrace) {
} on SocketException catch (error, stackTraceOrNull) {
// Work around issue 23008.
if (stackTrace == null) stackTrace = new Chain.current();
var stackTrace = stackTraceOrNull ?? new Chain.current();

if (error.osError == null) rethrow;

Expand Down
6 changes: 3 additions & 3 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ name: pub
dependencies:
# Note: Pub's test infrastructure assumes that any dependencies used in tests
# will be hosted dependencies.
analyzer: ">=0.25.0 <0.31.0"
analyzer: ^0.31.2-alpha.1
Copy link
Member

Choose a reason for hiding this comment

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

Are these tighter constraints really necessary? I generally try to keep constraints matching the actual APIs that are being used and let the version solver sort out which versions of the package are compatible with my Dart VM.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would you prefer ">=0.31.2-alpha.1 <0.32.0"?

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer >=0.25.0 <0.32.0.

Copy link
Member

Choose a reason for hiding this comment

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

There are ZERO cases we care about where the analyzer version != the version in the Dart SDK.

Let's do everything we can to make sure the version ran on CI and dev machines matches that...

Copy link
Member

Choose a reason for hiding this comment

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

@kevmoo The same argument could be made for every version constraint here, but setting a policy of constantly revving them to match the SDK's versions is untenable. We have thorough integration tests in the SDK to make sure the versions work out, and otherwise we should treat this like any other package and only muck with version constraints when it's actually necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

synced offline. I think that "works with Dart 2" is a new behavior for the package that we want to start depending on, and I think it makes sense for our pub constraint to reflect that.

In the end, as we discussed, for this package it's mostly academic.

args: ">=0.13.5 <2.0.0"
async: ">=1.12.0 <3.0.0"
async: ^2.0.0
collection: "^1.8.0"
crypto: ">=1.0.0 <3.0.0"
http: "^0.11.0"
Expand All @@ -19,7 +19,7 @@ dependencies:
path: "^1.2.0"
pool: "^1.0.0"
pub_semver: "^1.3.0"
shelf: ">=0.6.0 <0.7.0"
shelf: ^0.7.0
source_span: "^1.4.0"
stack_trace: "^1.0.0"
yaml: "^2.0.0"
Expand Down