Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Opening local dapp #4041

Merged
merged 3 commits into from
Jan 6, 2017
Merged

Opening local dapp #4041

merged 3 commits into from
Jan 6, 2017

Conversation

tomusdrw
Copy link
Collaborator

@tomusdrw tomusdrw commented Jan 4, 2017

Closes #4031

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 4, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 86.651% when pulling 1b66569 on parity-dapp into 9323704 on master.

@@ -97,6 +99,19 @@ fn read_manifest(name: &str, mut path: PathBuf) -> EndpointInfo {
})
}

pub fn local_endpoint(path: String, signer_address: Option<(String, u16)>) -> Option<(String, Box<LocalPageEndpoint>)> {
Copy link
Contributor

Choose a reason for hiding this comment

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

should accept path: PathBuf

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe even local_endpoint<P: AsRef<Path>>(path: P, ...) { ... }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

@rphmeier rphmeier added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 4, 2017
@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jan 5, 2017
@@ -97,9 +99,22 @@ fn read_manifest(name: &str, mut path: PathBuf) -> EndpointInfo {
})
}

pub fn local_endpoints(dapps_path: String, signer_address: Option<(String, u16)>) -> Endpoints {
pub fn local_endpoint<P: AsRef<Path>>(path: P, signer_address: Option<(String, u16)>) -> Option<(String, Box<LocalPageEndpoint>)> {
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to have short doc comments for non-trivial functions, even if they're non-visible outside their crates.

if let Some((id, endpoint)) = fs::local_endpoint(path.clone(), signer_address.clone()) {
pages.insert(id, endpoint);
} else {
warn!(target: "dapps", "Ingoring invalid dapp at {}", path.display());
Copy link
Contributor

Choose a reason for hiding this comment

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

*Ignoring

@gavofyork
Copy link
Contributor

typo and suggestion.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 6, 2017
@gavofyork gavofyork merged commit e983339 into master Jan 6, 2017
@gavofyork gavofyork deleted the parity-dapp branch January 6, 2017 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants