Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/ --- (Updated March 13, 2017, 6:19 p.m.) Review request for mesos, Alexander Rukletsov and Vinod Kone. Repository: mesos Description --- Followed the convention in 'master/http.cpp' and explicitly used the 'mesos::' prefix. Diffs - src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 Diff: https://reviews.apache.org/r/57341/diff/2/ Testing --- Thanks, Gastón Kleiman
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168801 --- Ship it! Ship It! - Alexander Rukletsov On March 8, 2017, 11:11 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 11:11 a.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
> On March 9, 2017, 9:20 p.m., Jie Yu wrote: > > What's the reason for this change? Can I get some context? > > Vinod Kone wrote: > I think Gaston wanted calls in slave/http.cpp to look similar to how they > were defined in master/http.cpp. The original inconsistency is because in > master/http.cpp, we needed the fully qualified namespace because compiler > gets confused between `mesos::internal::master` namespace and `mesos::master` > namespace. We didn't need it slave/http.cpp because the namespaces are > `mesos::internal::slave` and `mesos::agent`. That's right, I first wanted to fix inconsistencies in `slave/http.cpp` like the following: ``` case agent::Call::KILL_NESTED_CONTAINER: return killNestedContainer(call, mediaTypes.accept, principal); case mesos::agent::Call::LAUNCH_NESTED_CONTAINER_SESSION: return launchNestedContainerSession(call, mediaTypes, principal); ``` I looked at what we do in `master/http.cpp` in order to decide between `agent::Call` and `mesos::agent::Call`. - Gastón --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168523 --- On March 8, 2017, 11:11 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 11:11 a.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
> On March 9, 2017, 9:20 p.m., Jie Yu wrote: > > What's the reason for this change? Can I get some context? I think Gaston wanted calls in slave/http.cpp to look similar to how they were defined in master/http.cpp. The original inconsistency is because in master/http.cpp, we needed the fully qualified namespace because compiler gets confused between `mesos::internal::master` namespace and `mesos::master` namespace. We didn't need it slave/http.cpp because the namespaces are `mesos::internal::slave` and `mesos::agent`. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168523 --- On March 8, 2017, 11:11 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 11:11 a.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168523 --- What's the reason for this change? Can I get some context? - Jie Yu On March 8, 2017, 11:11 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 11:11 a.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
> On March 9, 2017, 2:20 p.m., Alexander Rojas wrote: > > I just wonder if there are any rules for when to use the namespace or when > > it can be omit it? I don't know if we have any rules for that. It just irked me that the usage of namespaces in this file wasn't consistent - neither with `master/http.cpp` nor within itself. - Gastón --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168448 --- On March 8, 2017, 11:11 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 11:11 a.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168448 --- Ship it! I just wonder if there are any rules for when to use the namespace or when it can be omit it? - Alexander Rojas On March 8, 2017, 12:11 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 12:11 p.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168390 --- Ship it! Ship It! - Vinod Kone On March 8, 2017, 11:11 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 11:11 a.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
> On March 8, 2017, 1:11 a.m., Kevin Klues wrote: > > This patch both does what the summary says AND adds the > > DELETE_NESTED_CONTAINER API call. Should we separate these into two > > separate patches? Yes, they belong in https://reviews.apache.org/r/57387/, thanks! - Gastón --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168228 --- On March 8, 2017, 11:11 a.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 8, 2017, 11:11 a.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/2/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/ --- (Updated March 8, 2017, 11:11 a.m.) Review request for mesos, Alexander Rukletsov and Vinod Kone. Changes --- Removed changes that don't belong to this patch. Bugs: MESOS-7120 https://issues.apache.org/jira/browse/MESOS-7120 Repository: mesos Description --- Followed the convention in 'master/http.cpp' and explicitly used the 'mesos::' prefix. Diffs (updated) - src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 Diff: https://reviews.apache.org/r/57341/diff/2/ Changes: https://reviews.apache.org/r/57341/diff/1-2/ Testing --- Thanks, Gastón Kleiman
Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168228 --- This patch both does what the summary says AND adds the DELETE_NESTED_CONTAINER API call. Should we separate these into two separate patches? - Kevin Klues On March 7, 2017, 5:15 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57341/ > --- > > (Updated March 7, 2017, 5:15 p.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7120 > https://issues.apache.org/jira/browse/MESOS-7120 > > > Repository: mesos > > > Description > --- > > Followed the convention in 'master/http.cpp' and explicitly used the > 'mesos::' prefix. > > > Diffs > - > > src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 > > > Diff: https://reviews.apache.org/r/57341/diff/1/ > > > Testing > --- > > > Thanks, > > Gastón Kleiman > >
Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/ --- Review request for mesos, Alexander Rukletsov and Vinod Kone. Bugs: MESOS-7120 https://issues.apache.org/jira/browse/MESOS-7120 Repository: mesos Description --- Followed the convention in 'master/http.cpp' and explicitly used the 'mesos::' prefix. Diffs - src/slave/http.cpp bc8209cb81194ebc8b604c9ba0d4a9e176cff2f6 Diff: https://reviews.apache.org/r/57341/diff/1/ Testing --- Thanks, Gastón Kleiman
