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

Clean up unhelpful extra folder names in crate libs #186

Closed
4 tasks
greg-szabo opened this issue Jul 31, 2020 · 3 comments
Closed
4 tasks

Clean up unhelpful extra folder names in crate libs #186

greg-szabo opened this issue Jul 31, 2020 · 3 comments
Labels
A: good-first-issue Admin: good for newcomers O: code-hygiene Objective: cause to improve code hygiene O: new-feature Objective: cause to add a new feature or support
Milestone

Comments

@greg-szabo
Copy link
Member

greg-szabo commented Jul 31, 2020

Summary

The modules crate's exported features are in an unnecessary long path.

For example in the CLI acceptance.rs file, here's how module components are imported:

use relayer_modules::ics03_connection::connection::ConnectionEnd;
use relayer_modules::ics03_connection::exported::Connection;
use relayer_modules::ics03_connection::exported::State as ConnectionState;
use relayer_modules::ics04_channel::channel::ChannelEnd;
use relayer_modules::ics04_channel::exported::Channel;
use relayer_modules::ics04_channel::exported::Order;
use relayer_modules::ics04_channel::exported::State as ChannelState;
use relayer_modules::ics23_commitment::CommitmentPrefix;
use relayer_modules::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
use relayer_modules::ics24_host::Path::{ChannelEnds, ClientConnections, Connections};

Proposal:

Remove the extra path where unnecessary, like exported, connection, etc.

For example, updated list:

use relayer_modules::ics03_connection::ConnectionEnd;
use relayer_modules::ics03_connection::Connection;
use relayer_modules::ics03_connection::State as ConnectionState;
use relayer_modules::ics04_channel::ChannelEnd;
use relayer_modules::ics04_channel::Channel;
use relayer_modules::ics04_channel::Order;
use relayer_modules::ics04_channel::State as ChannelState;
use relayer_modules::ics23_commitment::CommitmentPrefix;
use relayer_modules::ics24_host::{ChannelId, ClientId, ConnectionId, PortId};
use relayer_modules::ics24_host::Path::{ChannelEnds, ClientConnections, Connections};

(Makes easier to couple them too.)

To do this, the mod.rs file has to be updated. For example, instead of

pub mod connection;

it will read:

mod connection;
pub use connection::ConnectionEnd;

It's a bit of extra work for us since all published components need to be listed in the mod.rs file (unless there's some globbing option I'm not aware of), but it's beneficial for the developers: it reads more easily.

Please comment your opinions here. This is an opinionated idea, we may or may not want to do it.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@greg-szabo greg-szabo changed the title Clean up unhelpful Clean up unhelpful extra folder names in crate libs Jul 31, 2020
@adizere
Copy link
Member

adizere commented Jul 31, 2020

This is similar to what you did with ibc-proto. That was confusing to me initially, but I like the idea. Just a remark: This modification on its own can be confusing to newcomers. Specifically it is not intuitive what this trick achieves just by just reading mod.rs:

mod connection;
pub use connection::ConnectionEnd;

So I suggest at least a little comment in mod.rs to explain what's happening

The exported module brings up a related problem I believe. Parts of this module (and its intended functionality) may be unnecessary and we can remove it, but perhaps let's double check with @ancazamfir for confirmation. (Note that I removed parts of the ICS03 exported module already in separate work.)

@ancazamfir
Copy link
Collaborator

The exported module brings up a related problem I believe. Parts of this module (and its intended functionality) may be unnecessary and we can remove it..

yes, we should remove the traits from the exported.rs files (in both connection and channel folders).

@ancazamfir ancazamfir added this to the v0.0.4 milestone Aug 14, 2020
@ancazamfir ancazamfir modified the milestones: v0.0.4, v0.0.5 Sep 9, 2020
@adizere adizere mentioned this issue Sep 30, 2020
6 tasks
@adizere adizere added O: code-hygiene Objective: cause to improve code hygiene O: new-feature Objective: cause to add a new feature or support A: good-first-issue Admin: good for newcomers labels Sep 30, 2020
@ancazamfir ancazamfir modified the milestones: v0.0.5, v0.0.7 Oct 16, 2020
@adizere adizere modified the milestones: v0.1.0, v0.1.2 Jan 6, 2021
@romac
Copy link
Member

romac commented Nov 17, 2021

The exported namespace is not a thing anymore.

@romac romac closed this as completed Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: good-first-issue Admin: good for newcomers O: code-hygiene Objective: cause to improve code hygiene O: new-feature Objective: cause to add a new feature or support
Projects
None yet
Development

No branches or pull requests

4 participants