Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

plumbing/transport: export Transport impls #437

Closed
wants to merge 1 commit into from
Closed

plumbing/transport: export Transport impls #437

wants to merge 1 commit into from

Conversation

taralx
Copy link
Contributor

@taralx taralx commented Jun 18, 2017

Fixes #424

return &SCPEndpoint{
Username: m[1],
Hostname: m[2],
Pathname: m[3],
Copy link
Contributor

Choose a reason for hiding this comment

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

can you keep Pathname as Path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot, for the same reason as I needed to rename the other fields -- they conflict with the method names.

@taralx
Copy link
Contributor Author

taralx commented Jun 23, 2017

Ping?

@mcuadros mcuadros requested a review from smola June 28, 2017 15:09
Copy link
Collaborator

@smola smola left a comment

Choose a reason for hiding this comment

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

Looks good. These name prefix look pretty bad, but I can't think of a proper alternative.

@mcuadros
Copy link
Contributor

@taralx sorry about the delay, I am ok exporting the types, but you don't need to export the fields, since they can be accessed by the functions. This works for you?

@taralx
Copy link
Contributor Author

taralx commented Jul 31, 2017

@mcuadros In that case there's no advantage in exporting them, since they cannot be constructed.

@mcuadros
Copy link
Contributor

Can you explain your use case? Maybe you can create your own endpoint

@taralx
Copy link
Contributor Author

taralx commented Jul 31, 2017

It's #424 as specified in the commit message. If you don't want to export the fields of the others, that's fine by me -- I just don't know that it's worth exporting their types either if they're not constructible.

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.

3 participants