You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2013/10/17 23:02:50 UTC

Review Request 14722: Hard tabs in mesos

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

Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Repository: mesos-git


Description
-------

10 hard tabs found in mesos. Trivial patch converting hard tabs into spaces.


Diffs
-----

  src/detector/detector.cpp 12deefa 
  src/log/log.cpp aea06e7 
  src/master/master.cpp 1bf5d47 
  src/tests/allocator_tests.cpp 49fad2d 

Diff: https://reviews.apache.org/r/14722/diff/


Testing
-------

make check


Thanks,

Niklas Nielsen


Re: Review Request 14722: Hard tabs in mesos

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 17, 2013, 9:11 p.m., Ben Mahler wrote:
> > Ship It!

I missed some wrapping inconsistencies in the raw diff:

Here's some wrapping guidance that benh, vinod and I discussed a long time ago but never put in the style guide:

Declaring functions that can't fit on a single line.

Prefer this:
virtual Nothing resourcesUnused(
    const FrameworkID& frameworkId,
    const SlaveID& slaveId,
    const Resources& resources,
    const Option<Filters>& filters);

Over this:
virtual Nothing resourcesUnused(const FrameworkID& frameworkId,
                                const SlaveID& slaveId,
                                const Resources& resources,
                                const Option<Filters>& filters);

Calling functions:

(1)
allocator->resourcesUnused(frameworkId, slaveId, resources, filters);

(2)
allocator->resourcesUnused(frameworkId, slaveId,
                           resources, filters);

(3)
allocator->resourcesUnused(frameworkId,
                           slaveId,
                           resources,
                           filters);

(4)
allocator->resourcesUnused(
    frameworkId,
    slaveId,
    resources,
    filters);

(5)
allocator->resourcesUnused(
    frameworkId, slaveId, resources, filters);

Prefer (1), (4), (5), but never (2).

We generally void (3), but it can be used when it's not very jagged, for example:
foobar(someArgument,
       someOtherArgument,
       theLastArgument);


- Ben


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


On Oct. 17, 2013, 9:02 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14722/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2013, 9:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> 10 hard tabs found in mesos. Trivial patch converting hard tabs into spaces.
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 12deefa 
>   src/log/log.cpp aea06e7 
>   src/master/master.cpp 1bf5d47 
>   src/tests/allocator_tests.cpp 49fad2d 
> 
> Diff: https://reviews.apache.org/r/14722/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>


Re: Review Request 14722: Hard tabs in mesos

Posted by Niklas Nielsen <ni...@qni.dk>.

> On Oct. 17, 2013, 9:11 p.m., Ben Mahler wrote:
> > Ship It!
> 
> Ben Mahler wrote:
>     I missed some wrapping inconsistencies in the raw diff:
>     
>     Here's some wrapping guidance that benh, vinod and I discussed a long time ago but never put in the style guide:
>     
>     Declaring functions that can't fit on a single line.
>     
>     Prefer this:
>     virtual Nothing resourcesUnused(
>         const FrameworkID& frameworkId,
>         const SlaveID& slaveId,
>         const Resources& resources,
>         const Option<Filters>& filters);
>     
>     Over this:
>     virtual Nothing resourcesUnused(const FrameworkID& frameworkId,
>                                     const SlaveID& slaveId,
>                                     const Resources& resources,
>                                     const Option<Filters>& filters);
>     
>     Calling functions:
>     
>     (1)
>     allocator->resourcesUnused(frameworkId, slaveId, resources, filters);
>     
>     (2)
>     allocator->resourcesUnused(frameworkId, slaveId,
>                                resources, filters);
>     
>     (3)
>     allocator->resourcesUnused(frameworkId,
>                                slaveId,
>                                resources,
>                                filters);
>     
>     (4)
>     allocator->resourcesUnused(
>         frameworkId,
>         slaveId,
>         resources,
>         filters);
>     
>     (5)
>     allocator->resourcesUnused(
>         frameworkId, slaveId, resources, filters);
>     
>     Prefer (1), (4), (5), but never (2).
>     
>     We generally void (3), but it can be used when it's not very jagged, for example:
>     foobar(someArgument,
>            someOtherArgument,
>            theLastArgument);

Cool! Should be fixed in new patch.


- Niklas


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


On Oct. 17, 2013, 9:38 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14722/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2013, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> 10 hard tabs found in mesos. Trivial patch converting hard tabs into spaces.
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 12deefa 
>   src/log/log.cpp aea06e7 
>   src/master/master.cpp 1bf5d47 
>   src/tests/allocator_tests.cpp 49fad2d 
> 
> Diff: https://reviews.apache.org/r/14722/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>


Re: Review Request 14722: Hard tabs in mesos

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14722/#review27146
-----------------------------------------------------------

Ship it!


Ship It!

- Ben Mahler


On Oct. 17, 2013, 9:02 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14722/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2013, 9:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> 10 hard tabs found in mesos. Trivial patch converting hard tabs into spaces.
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 12deefa 
>   src/log/log.cpp aea06e7 
>   src/master/master.cpp 1bf5d47 
>   src/tests/allocator_tests.cpp 49fad2d 
> 
> Diff: https://reviews.apache.org/r/14722/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>


Re: Review Request 14722: Hard tabs in mesos

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14722/#review27165
-----------------------------------------------------------


Looks like the BasicMasterDetector constructors still have bad wrapping.

- Ben Mahler


On Oct. 17, 2013, 9:38 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14722/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2013, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> 10 hard tabs found in mesos. Trivial patch converting hard tabs into spaces.
> 
> 
> Diffs
> -----
> 
>   src/detector/detector.cpp 12deefa 
>   src/log/log.cpp aea06e7 
>   src/master/master.cpp 1bf5d47 
>   src/tests/allocator_tests.cpp 49fad2d 
> 
> Diff: https://reviews.apache.org/r/14722/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>


Re: Review Request 14722: Hard tabs in mesos

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14722/
-----------------------------------------------------------

(Updated Oct. 18, 2013, 4:04 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Changes
-------

Additional call wrappings.


Repository: mesos-git


Description
-------

10 hard tabs found in mesos. Trivial patch converting hard tabs into spaces.


Diffs (updated)
-----

  src/detector/detector.cpp 12deefa 
  src/log/log.cpp aea06e7 
  src/master/master.cpp 1bf5d47 
  src/tests/allocator_tests.cpp 49fad2d 

Diff: https://reviews.apache.org/r/14722/diff/


Testing
-------

make check


Thanks,

Niklas Nielsen


Re: Review Request 14722: Hard tabs in mesos

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14722/
-----------------------------------------------------------

(Updated Oct. 17, 2013, 9:38 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.


Changes
-------

Fix call wrapping


Repository: mesos-git


Description
-------

10 hard tabs found in mesos. Trivial patch converting hard tabs into spaces.


Diffs (updated)
-----

  src/detector/detector.cpp 12deefa 
  src/log/log.cpp aea06e7 
  src/master/master.cpp 1bf5d47 
  src/tests/allocator_tests.cpp 49fad2d 

Diff: https://reviews.apache.org/r/14722/diff/


Testing
-------

make check


Thanks,

Niklas Nielsen