> On March 26, 2014, 12:38 a.m., Vinod Kone wrote:
> > src/master/registrar.cpp, line 61
> > <https://reviews.apache.org/r/19657/diff/1/?file=536714#file536714line61>
> >
> >     Why is this not at the same place as Request and Response below?

This is because there is a mesos::Request and mesos::Response, I'll add a 
comment!


> On March 26, 2014, 12:38 a.m., Vinod Kone wrote:
> > src/master/registrar.cpp, line 98
> > <https://reviews.apache.org/r/19657/diff/1/?file=536714#file536714line98>
> >
> >     const?
> >     
> >     Also, why make it a method instead of just string like we did in 
> > master::http and slave::http?
> >     
> >     const static string REGISTRY_HELP;
> >     
> >     const string RegistrarProcess::REGISTRY_HELP = HELP();
> >

re: const

You cannot have a const static function, 'const' is reserved for member 
functions.


re: method vs static variable

We should avoid static non-POD variables because of the previous discussion 
around this:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Static_and_Global_Variables

We had a crash during process exit due to this.
Dominic brought this up and fixed several locations, but not master/slave http 
yet.


> On March 26, 2014, 12:38 a.m., Vinod Kone wrote:
> > src/master/registrar.cpp, line 162
> > <https://reviews.apache.org/r/19657/diff/1/?file=536714#file536714line162>
> >
> >     Can you show an example in the description like we did for monitor?

Sure! I wanted to originally but it was pretty big. Would be nice if stringify 
for JSON was for humans and we added a way to "serialize" JSON for computers 
(no whitespace). If we had this we could use mock protobufs to avoid needing to 
hand-write the JSON in HELP.


> On March 26, 2014, 12:38 a.m., Vinod Kone wrote:
> > src/master/registrar.cpp, lines 47-59
> > <https://reviews.apache.org/r/19657/diff/1/?file=536714#file536714line47>
> >
> >     These are not alphabetical.

Hm.. looks like we're inconsistent in this respect (master/http.cpp vs. 
status_update_manager.cpp). I was copying master/http.cpp but I'll update this.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19657/#review38562
-----------------------------------------------------------


On March 26, 2014, 12:22 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19657/
> -----------------------------------------------------------
> 
> (Updated March 26, 2014, 12:22 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Bugs: MESOS-764
>     https://issues.apache.org/jira/browse/MESOS-764
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added an endpoint for retrieving the registry contents.
> 
> 
> Diffs
> -----
> 
>   src/master/registrar.cpp ba39ad4a92aff97cb74c00f3a865ce670f22c29a 
> 
> Diff: https://reviews.apache.org/r/19657/diff/
> 
> 
> Testing
> -------
> 
> make check and hit the endpoint before a slave registered, after a slave 
> registered, after a slave was removed.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to