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

Change to machine config. #4

Merged
merged 13 commits into from
Jan 13, 2017
Merged

Change to machine config. #4

merged 13 commits into from
Jan 13, 2017

Conversation

wrathematics
Copy link
Contributor

As outlined in the first post of #3. I haven't updated the vignette yet, but the readme is up to date. Check it for an example.

The rpc() interface is now has to have a machine argument in it every time, but dealing with multiple machines is much simpler. It might be worth exporting some kind of set_default_machine(my_machine)-like function that sets .pbd_env$RPC.LI$machine or some such. Then rpc() (et al) can be modified so that the machine argument is at the end of the arguments list, and if left blank it would use the .pbd_env default. This would be nice if you had to deal with one machine a lot I suppose. Another option would be to add an R6 class (or even just fake it with a quick custom job of lists), giving a syntax like:

m <- Machine("my-host", "username", ...)
m$rpc("mycommand")

I get the feeling you don't like those interfaces, but I think they're cute. And if we use Machine() for the "class" constructor and machine() for the regular interface, we could have both at the same time.

@snoweye snoweye merged commit cd9fa9d into snoweye:master Jan 13, 2017
@snoweye
Copy link
Owner

snoweye commented Jan 13, 2017

See slack.

@snoweye
Copy link
Owner

snoweye commented Jan 13, 2017

You may want to do if(missing(machine)) so you can have a default when no machine is provided.

@snoweye
Copy link
Owner

snoweye commented Jan 13, 2017

I blindly merged and pushed, and I did not test rpc0() and your rpc() ... Please check carefully.

snoweye added a commit that referenced this pull request Aug 29, 2018
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.

2 participants