New RPC method "version". #835
Conversation
|
How about "best" instead of "recommended" to make the words roughly the same length? Just a thought. |
|
Cool idea, asking on the list... On Wed, Jan 28, 2015 at 11:46 AM, Vinnie Falco notifications@github.com
|
| @@ -0,0 +1,63 @@ | |||
| //------------------------------------------------------------------------------ | |||
There was a problem hiding this comment.
I'm not keen on having two files named Version.h that are one directory level away from each other.
There was a problem hiding this comment.
I'll rename rpc/Version to rpc/RPCVersion.h. (The other choice is renaming the version handler to VersionHandler, but we already have so many things marked as "Handler"...)
|
I've read over the document, but I'm still catching up on the actual implementation details. Let me see if I got this right.
As far as I can tell, we're not implementing any other part of the document besides adding the "version" RPC command. And that RPC command returns values, but accepts no settings. Is that correct? |
|
Regarding the #ifdef guard, I have an automatic tool that spits out .h files an ifdef that's guaranteed unique over all src files in rippled, so I'm just never going to look at it or change it, because I know it works. |
|
Scott, you are completely correct on all of these. The one unresolved question is the version number to give it. I'm proposing |
|
Pushed a new version with all those changes! |
|
Oh, one quibble - I decided to go with Vinnie's |
| template <class Object> | ||
| void setVersion(Object& parent) | ||
| { | ||
| auto&& object = addObject (parent, jss::version); |
There was a problem hiding this comment.
I have a lot to learn on the auto&& front. The way I read this, object may be an rvalue reference. Under what situation would that be desirable? Might it be better to have a plain auto& and have the compiler bark if addObject returned an rvalue?
|
@rec Can you adjust the settings of the automated tool to produce guards that more closely match the latest style in the code base? |
|
👍 |
| //------------------------------------------------------------------------------ | ||
| /* | ||
| This file is part of rippled: https://github.com/ripple/rippled | ||
| Copyright (c) 2012, 2013 Ripple Labs Inc. |
There was a problem hiding this comment.
It's 2015. The future!
|
Oh. You'll want to run |
|
(I fixed my boilerplate maker and released a little of it as an opensource tool... see other change lists...) |
| } | ||
|
|
||
| template <class Object> | ||
| void writeResult (Object& obj) |
There was a problem hiding this comment.
Should check() or writeResult() be static?
There was a problem hiding this comment.
Probably not since they are called through an instance of the class in the handler manager.
|
I guess we're done here. 👍 |
josh-ripple
added
the
Passed
|
Anyone else with thoughts? On Thu, Jan 29, 2015 at 4:33 AM, Josh Juran notifications@github.com
|
See https://docs.google.com/a/ripple.com/document/d/1LhD6DwFZvSVV-x4ebyKOdSp4oFDQEUXYYDzrau-ZP7E/edit#heading=h.x0obz18kye9x for more details.
Reviewers: @josh-ripple @scottschurr