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

Client IPC Interface #1493

Merged
merged 38 commits into from Jul 7, 2016
Merged

Client IPC Interface #1493

merged 38 commits into from Jul 7, 2016

Conversation

NikVolf
Copy link
Contributor

@NikVolf NikVolf commented Jun 29, 2016

IPC interface for Client

from example test to run client:

    let remote_client = nanoipc::init_client::<RemoteClient<_>>(socket_path).unwrap();
    assert!(remote_client.handshake().is_ok());

from example test to run worker:

    let mut worker = nanoipc::Worker::new(&client);
    worker.add_reqrep(&socket_path).unwrap();
    while !stop.load(Ordering::Relaxed) {
        worker.poll();
    }

Progress tracker

@NikVolf NikVolf added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Jun 29, 2016
@gavofyork gavofyork added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Jun 30, 2016
@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 5, 2016

@arkpar .rs.in -> .rs

@gavofyork gavofyork added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 6, 2016
@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Jul 6, 2016
@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 6, 2016

@gavofyork merged with master

@rphmeier
Copy link
Contributor

rphmeier commented Jul 6, 2016

still says there are merge conflicts

@arkpar
Copy link
Collaborator

arkpar commented Jul 6, 2016

File containing most of the clients code is called client_ipc.rs?
We do still want to have no-ipc build, right?
Client code should be ipc-independent ideally

@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 6, 2016

@rphmeier
because i have to merge conflicts every time someone else commits to the client.rs :)
merged again

@arkpar
everything is completely independent, if you create just Client, you work without any ipc
if you create RemoteClient, you work via ipc

@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jul 6, 2016
@gavofyork
Copy link
Contributor

i have to agree with @arkpar - why is Client class in a file called client_ipc.rs? unless there's good reasons, filenames should reflect the main class in them.

@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 6, 2016

@arkpar @gavofyork
right, didn't got the concerns right
actually extra file can be avoided completely
and diff if much cleaner now

})
.collect::<Vec<LocalizedLogEntry>>()
})
})
.collect::<Vec<LocalizedLogEntry>>()
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation messed up.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 6, 2016
@gavofyork
Copy link
Contributor

looks good apart from the errant indent.

@NikVolf
Copy link
Contributor Author

NikVolf commented Jul 6, 2016

@gavofyork fixed

@gavofyork gavofyork merged commit 8282c7d into master Jul 7, 2016
@gavofyork gavofyork deleted the client-ipc-refact branch July 7, 2016 07:39
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants