mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Avinash sridharan" <avin...@mesosphere.io>
Subject Re: Review Request 41617: Added a new category called whitespace/mesos-comments to capture missing, leading, white-space in comments
Date Fri, 15 Jan 2016 17:39:48 GMT


> On Jan. 14, 2016, 8:46 p.m., Michael Park wrote:
> > Please update the `Summary` to be < 72 chars to make Reviewbot happy. Could you
also submit a patch for the places that violate the rule we're enforcing here and make this
patch depend on it? Again, so that this way Reviewbot will be happy and our CI will continue
to pass.
> 
> Avinash sridharan wrote:
>     Re-indented to make comments fit in 72 lines.
> 
> Michael Park wrote:
>     Oh, I meant the `Summary` of this review: "Added a new category called whitespace/mesos-comments
to capture missing, leading, white-space in comments"
>     The `Summary` becomes the first line of the commit, and there's a pre-commit hook
to check that it's < 72 chars which fails Reviewbot:
>     
>     ```
>     Error: Commit message summary (the first line) must not exceed 72 characters.
>     ```

Ah got it. Was wondering why the reviewbot was still complaining since I had changed the commmit
message itself. Will change it.


- Avinash


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


On Jan. 15, 2016, 3:35 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41617/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2016, 3:35 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4231
>     https://issues.apache.org/jira/browse/MESOS-4231
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a new category called whitespace/mesos-comments to capture missing, leading, white-space
in comments
> 
> 
> Diffs
> -----
> 
>   support/cpplint.py 6890e27f92603b025e25e4db01decf351c33c9a1 
>   support/mesos-style.py 5ee17593153df7815426918a1da6e25cb1188bca 
> 
> Diff: https://reviews.apache.org/r/41617/diff/
> 
> 
> Testing
> -------
> 
> Ran the support/mesos-styles.py after this change and found zero errors.
> 
> Tried a commit for the following patch:
>   diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
> index eb47b53..e579d20 100644
> --- a/src/tests/slave_tests.cpp
> +++ b/src/tests/slave_tests.cpp
> @@ -2228,7 +2228,7 @@ TEST_F(SlaveTest, DiscoveryInfoAndPorts)
> 
>    AWAIT_READY(launchTask);
> 
> -  // Verify label key and value in slave state.json.
> +  //Verify label key and value in slave state.json.
>    Future<process::http::Response> response =
>      process::http::get(slave.get(), "state.json");
> 
> And the git commit hook correctly failed it:
> $ git commit -m "test"
> Checking 1 files
> src/tests/slave_tests.cpp:2231:  Should have a space between // and comment  [whitespace/comments]
[4]
> Total errors found: 1
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message