You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Yongqiao Wang <yq...@cn.ibm.com> on 2016/02/23 03:18:31 UTC
Review Request 43863: Move the implementation of updateWeights out of
header.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/
-----------------------------------------------------------
Review request for mesos, Adam B and Alexander Rukletsov.
Bugs: MESOS-4214
https://issues.apache.org/jira/browse/MESOS-4214
Repository: mesos
Description
-------
Move the implementation of updateWeights out of header.
Diffs
-----
src/CMakeLists.txt 5cf0ec8c475839ad8717192a37f01546cbcccd7a
src/Makefile.am 73e7ff06ba064c9b04f191009522d7808a7ab58e
src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
src/master/master.cpp b453bc7fca05c192df616b7d80132985b3248547
src/master/weights.hpp PRE-CREATION
src/master/weights.cpp PRE-CREATION
src/master/weights_handler.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/43863/diff/
Testing
-------
make && make check successfully!
Thanks,
Yongqiao Wang
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/
-----------------------------------------------------------
(Updated March 8, 2016, 1:05 p.m.)
Review request for mesos, Adam B and Alexander Rukletsov.
Changes
-------
Addressed comments of Adam.
Bugs: MESOS-4214
https://issues.apache.org/jira/browse/MESOS-4214
Repository: mesos
Description
-------
Move the implementation of updateWeights out of header.
Diffs (updated)
-----
src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4
src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261
src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8
src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f
src/master/weights.hpp PRE-CREATION
src/master/weights.cpp PRE-CREATION
src/master/weights_handler.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/43863/diff/
Testing
-------
make && make check successfully!
Thanks,
Yongqiao Wang
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/#review122490
-----------------------------------------------------------
Ship it!
Ship It!
- Adam B
On Feb. 27, 2016, 4:43 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2016, 4:43 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rukletsov.
>
>
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Move the implementation of updateWeights out of header.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 49a5645ef7242dbaee31e7b26dbbcb1f4f1f910e
> src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5
> src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
> src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1
> src/master/weights.hpp PRE-CREATION
> src/master/weights.cpp PRE-CREATION
> src/master/weights_handler.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43863/diff/
>
>
> Testing
> -------
>
> make && make check successfully!
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/#review122493
-----------------------------------------------------------
Fix it, then Ship it!
src/master/weights.hpp (line 35)
<https://reviews.apache.org/r/43863/#comment184522>
Kill the blank line
src/master/weights.hpp (line 60)
<https://reviews.apache.org/r/43863/#comment184524>
I like that you omit names for unused parameters, but please keep those, which are used (`registry`)! This serves as documentation.
src/master/weights.cpp (lines 51 - 52)
<https://reviews.apache.org/r/43863/#comment184526>
Blank line, please.
- Alexander Rukletsov
On Feb. 27, 2016, 12:43 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2016, 12:43 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rukletsov.
>
>
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Move the implementation of updateWeights out of header.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 49a5645ef7242dbaee31e7b26dbbcb1f4f1f910e
> src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5
> src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
> src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1
> src/master/weights.hpp PRE-CREATION
> src/master/weights.cpp PRE-CREATION
> src/master/weights_handler.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43863/diff/
>
>
> Testing
> -------
>
> make && make check successfully!
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/#review121088
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41597, 41681, 43863]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 27, 2016, 12:43 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2016, 12:43 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rukletsov.
>
>
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Move the implementation of updateWeights out of header.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 49a5645ef7242dbaee31e7b26dbbcb1f4f1f910e
> src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5
> src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
> src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1
> src/master/weights.hpp PRE-CREATION
> src/master/weights.cpp PRE-CREATION
> src/master/weights_handler.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43863/diff/
>
>
> Testing
> -------
>
> make && make check successfully!
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/
-----------------------------------------------------------
(Updated Feb. 27, 2016, 12:43 p.m.)
Review request for mesos, Adam B and Alexander Rukletsov.
Changes
-------
Addressed the comments of Adam.
Bugs: MESOS-4214
https://issues.apache.org/jira/browse/MESOS-4214
Repository: mesos
Description
-------
Move the implementation of updateWeights out of header.
Diffs (updated)
-----
src/CMakeLists.txt 49a5645ef7242dbaee31e7b26dbbcb1f4f1f910e
src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5
src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1
src/master/weights.hpp PRE-CREATION
src/master/weights.cpp PRE-CREATION
src/master/weights_handler.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/43863/diff/
Testing
-------
make && make check successfully!
Thanks,
Yongqiao Wang
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/
-----------------------------------------------------------
(Updated Feb. 27, 2016, 12:40 p.m.)
Review request for mesos, Adam B and Alexander Rukletsov.
Changes
-------
Addressed the comments of Adam.
Bugs: MESOS-4214
https://issues.apache.org/jira/browse/MESOS-4214
Repository: mesos
Description
-------
Move the implementation of updateWeights out of header.
Diffs (updated)
-----
src/CMakeLists.txt 49a5645ef7242dbaee31e7b26dbbcb1f4f1f910e
src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5
src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1
src/master/weights.hpp PRE-CREATION
src/master/weights.cpp PRE-CREATION
src/master/weights_handler.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/43863/diff/
Testing
-------
make && make check successfully!
Thanks,
Yongqiao Wang
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/#review120282
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41597, 41681, 43863]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 23, 2016, 2:18 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2016, 2:18 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rukletsov.
>
>
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Move the implementation of updateWeights out of header.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 5cf0ec8c475839ad8717192a37f01546cbcccd7a
> src/Makefile.am 73e7ff06ba064c9b04f191009522d7808a7ab58e
> src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
> src/master/master.cpp b453bc7fca05c192df616b7d80132985b3248547
> src/master/weights.hpp PRE-CREATION
> src/master/weights.cpp PRE-CREATION
> src/master/weights_handler.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43863/diff/
>
>
> Testing
> -------
>
> make && make check successfully!
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Yongqiao Wang <yq...@cn.ibm.com>.
> On Feb. 27, 2016, 11:17 a.m., Adam B wrote:
> > Looks good to me. I'm assuming you didn't change anything substantial in the implementation when you moved it. It looks the same to me.
Yes, I didn't change anything substantial in the implementation.
> On Feb. 27, 2016, 11:17 a.m., Adam B wrote:
> > src/Makefile.am, lines 630-631
> > <https://reviews.apache.org/r/43863/diff/1/?file=1264901#file1264901line630>
> >
> > Do these need to be in separate files?
Yes, I think weights_handler.cpp only contains the logic to handle the http request, like quota and maintenance features, they all use the separated files. Maybe my mainly reason is better to keep consistent.
- Yongqiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/#review121078
-----------------------------------------------------------
On Feb. 23, 2016, 2:18 a.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2016, 2:18 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rukletsov.
>
>
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Move the implementation of updateWeights out of header.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 5cf0ec8c475839ad8717192a37f01546cbcccd7a
> src/Makefile.am 73e7ff06ba064c9b04f191009522d7808a7ab58e
> src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
> src/master/master.cpp b453bc7fca05c192df616b7d80132985b3248547
> src/master/weights.hpp PRE-CREATION
> src/master/weights.cpp PRE-CREATION
> src/master/weights_handler.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43863/diff/
>
>
> Testing
> -------
>
> make && make check successfully!
>
>
> Thanks,
>
> Yongqiao Wang
>
>
Re: Review Request 43863: Move the implementation of updateWeights
out of header.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43863/#review121078
-----------------------------------------------------------
Fix it, then Ship it!
Looks good to me. I'm assuming you didn't change anything substantial in the implementation when you moved it. It looks the same to me.
src/Makefile.am (lines 630 - 631)
<https://reviews.apache.org/r/43863/#comment182669>
Do these need to be in separate files?
src/master/weights.hpp (line 60)
<https://reviews.apache.org/r/43863/#comment182671>
This fits on one line if you exclude variable names for the fields you don't use, e.g. ` Try<bool> perform(Registry* registry, hashset<SlaveID>*, bool);`
src/master/weights.cpp (line 25)
<https://reviews.apache.org/r/43863/#comment182672>
Only indent 2 spaces
- Adam B
On Feb. 22, 2016, 6:18 p.m., Yongqiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 6:18 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rukletsov.
>
>
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Move the implementation of updateWeights out of header.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 5cf0ec8c475839ad8717192a37f01546cbcccd7a
> src/Makefile.am 73e7ff06ba064c9b04f191009522d7808a7ab58e
> src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d
> src/master/master.cpp b453bc7fca05c192df616b7d80132985b3248547
> src/master/weights.hpp PRE-CREATION
> src/master/weights.cpp PRE-CREATION
> src/master/weights_handler.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43863/diff/
>
>
> Testing
> -------
>
> make && make check successfully!
>
>
> Thanks,
>
> Yongqiao Wang
>
>