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

Integrate with Routing #888

Merged
merged 2 commits into from Nov 11, 2019

Conversation

@nbaksalyar
Copy link
Member

nbaksalyar commented Oct 31, 2019

No description provided.

@nbaksalyar nbaksalyar referenced this pull request Oct 31, 2019
@nbaksalyar nbaksalyar force-pushed the nbaksalyar:routing-integration branch from 58b9eaa to 5cd5e72 Nov 7, 2019
@nbaksalyar nbaksalyar changed the title (WIP) Integrate with Routing Integrate with Routing Nov 7, 2019
@nbaksalyar nbaksalyar marked this pull request as ready for review Nov 7, 2019
@nbaksalyar nbaksalyar requested a review from ustulation as a code owner Nov 7, 2019
@@ -169,14 +189,48 @@ impl Vault {
pub fn poll(&mut self) -> bool {

This comment has been minimized.

Copy link
@ustulation

ustulation Nov 8, 2019

Member

Why do we still need this if run() is fully functional now ?

This comment has been minimized.

Copy link
@nbaksalyar

nbaksalyar Nov 8, 2019

Author Member

We don't need it for the production code, but a lot of things in the tests depend on it.
So probably it would be the best to put it behind a feature flag (as #[cfg(test)]). What do you think?

This comment has been minimized.

Copy link
@ustulation

ustulation Nov 11, 2019

Member

Hmm - this is not doable as the integration tests call it too so needs to be either behind a custom feature flag or just left as is for now. I'm leaving it as is and we can discuss it later.

Copy link
Member

lionel1704 left a comment

Everything looks good to me. Just a couple of very minor changes.

}

impl Vault {
/// Construct a new vault instance.
/// Create and start vault. This will block unitl a `Command` to free it is fired.

This comment has been minimized.

Copy link
@lionel1704

lionel1704 Nov 11, 2019

Member

until* :)

#[cfg(feature = "mock")]
pub use self::mock::quic_p2p;
#[cfg(not(feature = "mock"))]
pub use quic_p2p;
// FIXME: uncomment once we have compatible Routing API.

This comment has been minimized.

Copy link
@lionel1704

lionel1704 Nov 11, 2019

Member

This FIXME can be removed now.

ustulation added a commit to ustulation/safe_vault that referenced this pull request Nov 11, 2019
@ustulation ustulation merged commit 5cd5e72 into maidsafe:vNext Nov 11, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/jenkins/pr-merge This commit cannot be built
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
@nbaksalyar nbaksalyar deleted the nbaksalyar:routing-integration branch Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.