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

refactor!: Reorganize exports and modules exposed #102

Merged
merged 7 commits into from
Apr 4, 2022

Conversation

austinabell
Copy link
Contributor

I'll try to summarize all changes:

  • Export network module publicly, which includes the specific network implementations (it is necessary for these to be exported since it's part of public API)
  • Move result module to root level (non-breaking, was exported before)
  • Move network agnostic types to the types directory (non-breaking)
  • Remove StatePatcher and AllowStatePatcher traits. Ideally, this came in separate PR, but didn't want to export them just to remove later
  • Create operations module. I couldn't think of a better name but these builder types need to be exported since part of the public API. Decided to include the breaking change of removing the root declaration of Function, but this can be re-exported if we think it should be root level

I don't think this is perfect, but moves towards exports being a little more organized for maintainability but also so that the types exposed on our public API are exported.

@@ -3,10 +3,11 @@ use std::path::PathBuf;
use async_trait::async_trait;

use crate::network::Info;
use crate::network::{Account, CallExecution, NetworkClient, NetworkInfo, TopLevelAccountCreator};
use crate::network::{NetworkClient, NetworkInfo, TopLevelAccountCreator};
use crate::result::CallExecution;

Choose a reason for hiding this comment

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

Why is this type not exposed? It is returned by exposed functions. So if a user wants to make a function that returns the result of one of the exposed function they reference the type.

Copy link
Contributor Author

@austinabell austinabell Apr 4, 2022

Choose a reason for hiding this comment

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

What are you referring to? This is just a type that wraps the type returned that includes metadata.

Edit: Also confused on what you're asking. It is exposed

Choose a reason for hiding this comment

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

Sorry I meant from the crate: https://github.com/near/workspaces-rs/pull/102/files#diff-f2509a15c16bf20cbd905fc2d19c6b77363687408bf1020327da68936ec19ef9

Account exposes CallExecutionDetails and CallExecution, but they aren't exported.

Copy link
Member

Choose a reason for hiding this comment

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

those are exposed under the result module which we have exported via pub mod result; in lib.rs

Choose a reason for hiding this comment

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

Ah I see thanks!

Copy link
Member

@ChaoticTempest ChaoticTempest left a comment

Choose a reason for hiding this comment

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

This moves towards some pretty clean exports! Thanks for getting this in. Also, #73 already removed StatePatcher but it's fine, we can fix the merge conflicts if there ends up becoming one

Copy link
Member

@ChaoticTempest ChaoticTempest left a comment

Choose a reason for hiding this comment

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

new changes lgtm

@ChaoticTempest ChaoticTempest merged commit 5fe9cb8 into main Apr 4, 2022
@ChaoticTempest ChaoticTempest deleted the austin/refactor_exports branch April 4, 2022 22:54
@austinabell austinabell mentioned this pull request Apr 5, 2022
@frol frol mentioned this pull request Oct 4, 2023
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.

3 participants