Skip to content

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Feb 13, 2019

Relands #26944 (was reverted in #27191)

Multicast_dns has landed internally, so this should no longer be an internal roll blocker.

/cc @zanderso who was cc'd on the revert for reasons I'm not fully aware of :)

fixes #23164

@jonahwilliams
Copy link
Contributor

Generally LGTM - how do you feel about the refactor in #27455 ? think the attach command is growing a bit long, and the is checks are a definite smell


import 'dart:async';

import 'package:multicast_dns/multicast_dns.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this will need to be added to the BUILD.gn as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added - can you verify I did it right though?

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @zanderso do we need to manually roll this into fuchsia?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, after the in-flight roll lands, this will cause the auto-roller to block until Fuchsia's third_party Dart packages are manually updated. I can help with that.

Also, it should go in this one https://github.com/flutter/flutter/blob/master/packages/flutter_tools/BUILD.gn, not the one for flutter_test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that. I've moved it to the correct BUILD.gn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this good to land now, or were we waiting on confirmation for any of this?

Copy link
Member

Choose a reason for hiding this comment

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

This is okay to land from the Fuchsia perspective. An earlier flutter/flutter change also requires an update to the Fuchsia third_party Dart packages, so it would be convenient to handle this one and that one at the same time.

@zoechi zoechi added the tool Affects the "flutter" command-line tool. See also t: labels. label Feb 14, 2019
@dnfield dnfield added platform-ios iOS applications specifically platform-mac Building on or for macOS specifically labels Feb 14, 2019
@dnfield
Copy link
Contributor Author

dnfield commented Feb 15, 2019

Note: This commit is pinning json_schema to 1.0.10, since >=2.0.0 adds additional transitive dependencies we don't want in the tools. Opened #28021 to track.

@dnfield dnfield merged commit c90c3a1 into flutter:master Feb 16, 2019
@dnfield dnfield deleted the ios_attach branch February 16, 2019 01:45
@Hixie
Copy link
Contributor

Hixie commented Feb 19, 2019

I don't understand how, but this apparently raised our consumer facing dependency count by 4 to 35.
This seems problematic on two fronts. One, raising our dependency count by >10% is pretty serious. Two, why does the patch show no evidence of this?

@jonahwilliams
Copy link
Contributor

I think we have a bug in our dependency pinning script, it is automatically including pinned packages in the transitive closure

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

platform-ios iOS applications specifically platform-mac Building on or for macOS specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Teach flutter_tools how to use mDNS to find the iOS observatory port

6 participants