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

add GetVersions() as an interface method for exported.ConnecionI #3786

Closed
charleenfei opened this issue Jun 7, 2023 · 4 comments
Closed
Labels
03-connection core type: code hygiene Clean up code but without changing functionality or interfaces
Milestone

Comments

@charleenfei
Copy link
Contributor

currently, we need to get an instance of ConnectionEnd in order to use GetVersions()

should we add GetVersions() as an interface method to exported.ConnectionI? cc. @colin-axner

Originally posted by @damiannolan in #3774 (comment)

@damiannolan
Copy link
Member

Awesome, thanks for opening this issue @charleenfei. We can update the code from where I made the comment on the 04-channel-upgrades branch once this is changed :)

We can make this to main and then resync the feat branch whenever 👍

@damiannolan damiannolan added core type: code hygiene Clean up code but without changing functionality or interfaces 03-connection labels Jun 7, 2023
@damiannolan damiannolan added this to the v8.0.0 milestone Jun 7, 2023
@crodriguezvega
Copy link
Contributor

This is the issue that we had in the past to remove GetVersions from the interface.

@colin-axner
Copy link
Contributor

I think we can just modify channelKeeper.GetConnection() to return the concrete connection type? I see connection types is imported by channel types, so I'm not sure what import cycle is being avoided using exported.

The usage of interface to avoid import cycles is quite dated. We didn't think it through too much at the time for the sake of delivering IBC faster, so I think it's worth trying to remove some of these unnecessary interfaces

@crodriguezvega
Copy link
Contributor

If that's ok, I will close this for now, since we decided (and already implemented) to use the connection keeper to get the connection (which returns the concrete type), so this should not be needed anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
03-connection core type: code hygiene Clean up code but without changing functionality or interfaces
Projects
Status: Done 🥳
Development

No branches or pull requests

4 participants