Skip to content

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Jun 1, 2017

This change passes the address from the command line to ssh. It could instead get the ipv6 address from netaddr --fuchsia, but I haven't been able to get that to work with my setup.

@zanderso
Copy link
Member Author

zanderso commented Jun 1, 2017

/cc @abarth

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the syntax for TODOs is // TODO(username): message

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@Hixie
Copy link
Contributor

Hixie commented Jun 12, 2017

cc @tvolkert

Copy link
Contributor

Choose a reason for hiding this comment

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

Use processManager.run() so this can be mocked out in tests.

Then, can you add a test? :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@tvolkert tvolkert left a comment

Choose a reason for hiding this comment

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

LGTM

@zanderso zanderso merged commit 09bdab2 into flutter:master Jun 23, 2017
@zanderso zanderso deleted the fuchsia-reload-ssh branch June 23, 2017 05:08
@Hixie
Copy link
Contributor

Hixie commented Jun 23, 2017

BTW, this was checked in when the tree was red. Please avoid checking anything in when the tree is red except for attempts to fix the tree, because it makes it much harder to detect, and later identify, additional failures introduced while the previous failure was ongoing. Thanks!

gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jul 1, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants