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

Split and move xfer package. #794

Merged
merged 1 commit into from
Jan 6, 2016
Merged

Split and move xfer package. #794

merged 1 commit into from
Jan 6, 2016

Conversation

tomwilkie
Copy link
Contributor

Fixes #641

@tomwilkie tomwilkie force-pushed the 641-xfer-mv branch 2 times, most recently from e9900b0 to ba21e24 Compare January 5, 2016 10:13
@paulbellamy
Copy link
Contributor

Not a fan of:

import (
  "github.com/weaveworks/weave/common"
  xcommon "github.com/weaveworks/scope/common/xfer" // sometimes called just `common`
  "github.com/weaveworks/scope/probe/xfer"
)

Maybe:
probeXfer "github.com/weaveworks/scope/probe/xfer", or
commonXfer "github.com/weaveworks/scope/common/xfer"

In general common "github.com/weaveworks/scope/common/xfer" seems a bit confusing, since common is already the name of another package.

@tomwilkie
Copy link
Contributor Author

@paulbellamy I agree (x)common is pretty ugly; but the cAmElCaSe seems pretty ugly too. How about we rename probe/xfer to be proble/appclient?

@paulbellamy
Copy link
Contributor

probe/xfer -> app/client ?

Edit: or probe/appclient, either one.

@tomwilkie
Copy link
Contributor Author

@paulbellamy PTAL

paulbellamy added a commit that referenced this pull request Jan 6, 2016
@paulbellamy paulbellamy merged commit 80c7436 into master Jan 6, 2016
@paulbellamy paulbellamy deleted the 641-xfer-mv branch January 6, 2016 15:56
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