Skip to content
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

A few minor updates to the tracing interface. #2081

Merged
merged 1 commit into from
Jul 30, 2018

Conversation

AWoloszyn
Copy link
Contributor

This allows you to specify environment variables in your
ssh config, (instead of having to add them every time).

It also exposes portWatcher for external use.

Copy link
Contributor

@ben-clayton ben-clayton left a comment

Choose a reason for hiding this comment

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

Build is failing as you need to re-run gazelle.

@@ -29,7 +29,17 @@ import (

var portPattern = regexp.MustCompile(`^Bound on port '(\d+)'$`)

type portWatcher struct {
func NewPortWatcher(portChan chan<- string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: No need to break this signature across two lines

}
}

type PortWatcher struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Godocs please

@@ -144,7 +141,7 @@ func (t *DesktopTracer) GetTraceTargetNode(ctx context.Context, uri string, icon
children[i] = filepath.Join(uri, children[i])
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 you need to deal with the target device slashes here too?

This allows you to specify environment variables in your
ssh config, (instead of having to add them every time).

It also exposes portWatcher for external use.
@AWoloszyn AWoloszyn merged commit 751cc96 into google:master Jul 30, 2018
@AWoloszyn AWoloszyn deleted the trace-updates branch July 30, 2018 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants