> On May 5, 2016, 8:49 p.m., Michael Park wrote: > > 3rdparty/libprocess/include/process/http.hpp, lines 584-603 > > <https://reviews.apache.org/r/46504/diff/2/?file=1369337#file1369337line584> > > > > Is this a strict format? That is, are we required to phraes it this > > way? Can we just take `allowMethods` by value, and do the following? > > > > ``` > > foreach (std::string& method, allowMethods) { > > method = "'" + method + "'"; > > } > > > > return "Expecting one of: " + strings::join(", ", allowMethods) + > > ", but received '" + requestMethod + "' "; > > ```
I think we definitely should change the format of the messge and simplify the code! I personally would wrap allowed methods in curly braces for clarity; other than that MPark's suggestion seems fine. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46504/#review131919 ----------------------------------------------------------- On May 3, 2016, 12:20 a.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46504/ > ----------------------------------------------------------- > > (Updated May 3, 2016, 12:20 a.m.) > > > Review request for mesos and Alexander Rukletsov. > > > Bugs: MESOS-4126 > https://issues.apache.org/jira/browse/MESOS-4126 > > > Repository: mesos > > > Description > ------- > > Constructed error string in MethodNotAllowed. > > - Decided to use constructor delegation rather than just constructing in > MethodNotAllowed > allowing body to be constructed before calling the Response constructor. > This in addition > to the improved readability and sectioning off the verbose body > construction as a middle step > in a delegation chain led me to this implementation. Noting here that > construction in > MethodNotAllowed would be completely valid, but seeing what others think of > this choice. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/http.hpp > 8f4eabcbb71ead1f5c28e1d8a2dd40db7af1f297 > > Diff: https://reviews.apache.org/r/46504/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jacob Janco > >