Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Kevin Klues]

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


Ship it!




Thanks Yong!

Patch looks great now.  The only thing I'd change at this point is the wording 
in the commit message.

Probably something like:
```
Add a commit hook for checking non-printable characters (MESOS-4033).

This patch adds an addition check in mesos-style.pl to check
for non-printable characters. It scans .cpp, .hpp, .cc, .h
files and reports an error if non-printable characters exist.

As part of this patch, two non-printable characters have been identified
in versioning.md (one in Line 85 and another in Line 96) and are corrected
accordingly.

Note: Scanning .md files is skipped based on feedback from reviews.

Note: This commit includes patches from Kevin Klues and haosdent.
```

Thanks!

- Kevin Klues


On April 3, 2016, 8:36 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45033]

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 April 3, 2016, 8:36 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On April 3, 2016, 7:51 p.m., Kevin Klues wrote:
> > support/mesos-style.py, line 108
> > 
> >
> > I might have slipped it in a little late, but I also had a comment 
> > about the actual matching going on here.
> > 
> > This match doesn't exclude non-printable characters from the lower 127 
> > ascii codes, nor does it exclude unicode characters (which I think we want 
> > to exclude).
> > 
> > I updated the patch I referenced before to include this.
> 
> Yong Tang wrote:
> Thanks Kevin. Let me take a look and get this issue addressed.

Thanks Kevin. I updated the review request with the latest changes. As part of 
MESOS-4033, I scan through the current source code and there are several .cpp 
and .hpp files that consists of non-printable characters. They are:

3rdparty/libprocess/3rdparty/stout/include/stout/windows/format.hpp

src/slave/flags.cpp
src/uri/fetchers/curl.cpp
src/uri/fetchers/docker.cpp

I have to created two additional commits to change those non printable chars 
into printable chars, as the current commit hook will not allow me to check in 
code that across both libprocess and mesos. Those two commits are located:
https://reviews.apache.org/r/45659/
https://reviews.apache.org/r/45660/

Please take a look and let me know if there are any issues.


- Yong


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


On April 3, 2016, 8:36 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On April 3, 2016, 7:51 p.m., Kevin Klues wrote:
> >

Yes string.printable is a better way of handling this issue. Let me update the 
review request shortly.


- Yong


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


On April 3, 2016, 7:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On April 3, 2016, 7:51 p.m., Kevin Klues wrote:
> > support/mesos-style.py, line 108
> > 
> >
> > I might have slipped it in a little late, but I also had a comment 
> > about the actual matching going on here.
> > 
> > This match doesn't exclude non-printable characters from the lower 127 
> > ascii codes, nor does it exclude unicode characters (which I think we want 
> > to exclude).
> > 
> > I updated the patch I referenced before to include this.

Thanks Kevin. Let me take a look and get this issue addressed.


- Yong


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


On April 3, 2016, 7:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Kevin Klues]

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




support/mesos-style.py (line 108)


I might have slipped it in a little late, but I also had a comment about 
the actual matching going on here.

This match doesn't exclude non-printable characters from the lower 127 
ascii codes, nor does it exclude unicode characters (which I think we want to 
exclude).

I updated the patch I referenced before to include this.


- Kevin Klues


On April 3, 2016, 7:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On April 3, 2016, 4:21 p.m., Kevin Klues wrote:
> > Thanks Yong for all your work on this!
> > 
> > Just a few comments about style and one comment regarding the use of 
> > `re.match()` instead of `re.search()`.
> > 
> > Please see my patch on github which applies all of the comments I've left:
> > https://github.com/klueska-mesosphere/mesos/commits/r/yongtang/non-ascii
> > 
> > Thanks!
> 
> haosdent huang wrote:
> Seems our reviews went into thread competition in real life. Sorry to 
> forgot check this before I public my comments.

Thanks a lot for all the detailed reviews, Kevin and haosdent. And I really 
appreciate that. The review request has been updated. Please let me know if 
there are any additional issues. Again, thanks a lot for all the efforts!


- Yong


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


On April 3, 2016, 7:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Kevin Klues]

> On April 3, 2016, 4:21 p.m., Kevin Klues wrote:
> > support/mesos-style.py, line 43
> > 
> >
> > This will not work with `re.match()`. As is, this will only match if 
> > the non-ascii character is the first character on the line.  Instead, you 
> > should probably be using `re.search()`.
> > 
> > Also, the variable `m` should probably be renamed `match` and spaces 
> > should be added around the `=`.
> 
> Kevin Klues wrote:
> Also, I'm not sure if this is actually the correct match to be looking 
> for.  What we basically want is a check to see if the characters are 
> printable characters between 0-127. This will **not** exclude unicode 
> characters, nor characters in the rhange 0-127 which are not printable (e.g. 
> 'bell').

I've updated my the patch at 
https://github.com/klueska-mesosphere/mesos/commits/r/yongtang/non-ascii to 
reflect, what I think is probably a better method for determining the 
non-printable characters.

```
# If we find an error, add 1 to both the character and
# the line offset to give them 1-based indexing   
# instead of 0 (as is common in most editors).
char_errors = [offset + 1 for offset, char in enumerate(line) 
   if char not in string.printable]   
if char_errors:   
sys.stderr.write( 
"Non printable characters found in {path} "   
 "(Line: {line_number}, Chars: {chars}): {line}".format( 
 path=path,
 line_number=line_number + 1,  
 chars=char_errors,
 line=line))   
```


- Kevin


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


On April 3, 2016, 7:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 3, 2016, 7:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> Note: This commit includes patches from Kevin Klues and haosdent.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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

(Updated April 3, 2016, 7:46 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi Xiao.


Changes
---

Update review request with patches from Kevin Klues and feedback from haosdent.


Bugs: MESOS-4033
https://issues.apache.org/jira/browse/MESOS-4033


Repository: mesos


Description (updated)
---

This review request tries to add addition check in mesos-style.pl
for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
files and report error if non-ascii characters exists.

As part of this review request, two non-ascii characters are identified
in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
accordingly.

Note: .md scan is skipped based on feedback from review request.

Note: This commit includes patches from Kevin Klues and haosdent.


Diffs (updated)
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Kevin Klues]

> On April 3, 2016, 4:33 p.m., haosdent huang wrote:
> > support/mesos-style.py, line 47
> > 
> >
> > Actually you could use 
> > 
> > ```
> > for line_no, line in enumerate(source_file):
> >   ...
> > ```
> > 
> > so that you don't need this wired way here.

Yes, this is better (though I'd still call it line_number, not line_no). Don't 
forget to do a +1 too to account for line_number being indexed at 0 when 
printing in the error string.


- Kevin


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


On April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Kevin Klues]

> On April 3, 2016, 4:21 p.m., Kevin Klues wrote:
> > support/mesos-style.py, line 43
> > 
> >
> > This will not work with `re.match()`. As is, this will only match if 
> > the non-ascii character is the first character on the line.  Instead, you 
> > should probably be using `re.search()`.
> > 
> > Also, the variable `m` should probably be renamed `match` and spaces 
> > should be added around the `=`.

Also, I'm not sure if this is actually the correct match to be looking for.  
What we basically want is a check to see if the characters are printable 
characters between 0-127. This will **not** exclude unicode characters, nor 
characters in the rhange 0-127 which are not printable (e.g. 'bell').


- Kevin


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


On April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[haosdent huang]

> On April 3, 2016, 4:33 p.m., haosdent huang wrote:
> > support/mesos-style.py, line 147
> > 
> >
> > How about use `no_ascii_errors`?

encoding_errors as @klueska said more meaningful. Let me drop this.


- haosdent


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


On April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[haosdent huang]

> On April 3, 2016, 4:21 p.m., Kevin Klues wrote:
> > Thanks Yong for all your work on this!
> > 
> > Just a few comments about style and one comment regarding the use of 
> > `re.match()` instead of `re.search()`.
> > 
> > Please see my patch on github which applies all of the comments I've left:
> > https://github.com/klueska-mesosphere/mesos/commits/r/yongtang/non-ascii
> > 
> > Thanks!

Seems our reviews went into thread competition in real life. Sorry to forgot 
check this before I public my comments.


- haosdent


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


On April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[haosdent huang]

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



@yongtang, Thank you very much for your patch and follow up continuously! The 
patch looks good to me and I left my personal comments on it. Feel free to drop 
it if you have better ideas.


support/mesos-style.py (line 39)


I think use 
```
for path in source_paths
```
better



support/mesos-style.py (line 41)


Variable name and space
```
line_no = 1
```



support/mesos-style.py (line 42)


I suggest use `source_file` instead of `p`. `p` looks confusing here.



support/mesos-style.py (line 43)


Need add space between `=`
```
m = re.match...
```



support/mesos-style.py (line 45)


Add space between `+` as well. By the way, this line overflow 80 characters 
limit? I think could break this line to few lines and add some extra local 
variables.

And I think `Ln` and `Pos` is unclear, I suggest use another clear name.

And `m.start() + 1` is because of you want to the base of index is start 
from `1` in display message?



support/mesos-style.py (line 47)


Actually you could use 

```
for line_no, line in enumerate(source_file):
  ...
```

so that you don't need this wired way here.



support/mesos-style.py (line 147)


How about use `no_ascii_errors`?


- haosdent huang


On April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Kevin Klues]

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



Thanks Yong for all your work on this!

Just a few comments about style and one comment regarding the use of 
`re.match()` instead of `re.search()`.

Please see my patch on github which applies all of the comments I've left:
https://github.com/klueska-mesosphere/mesos/commits/r/yongtang/non-ascii

Thanks!


support/mesos-style.py (line 34)


We should probably call this `check_encoding()` instead of `run_ascii()` to 
be more consistent with `check_license_header()`.

The `run_lint()` function actually calls out to a linter, so this name was 
more appropriate here.

Also, `check_encoding()` is probably better than `check_ascii()` because it 
is a bit clearer as to our intent.



support/mesos-style.py (lines 35 - 37)


We should probably have a bit longer description here, making it clear that 
this covers both traditional ascii characters (0-127) as well as the extended 
ascii characters (128-255). I would probably even link to one of the ascii 
tables online.



support/mesos-style.py (line 38)


We typically put spaces between our variable names and our equal signs. 
e.g.:

```
total_errors = 0
```

Also, we should probably name this variable `error_count` instead of 
`total_errors` to be consistent with `check_license_header()`.

In general, this function should probably follow the same structure as 
`check_license_header()` except for the regex.



support/mesos-style.py (lines 39 - 40)


We should probably use the same variable names as `check_license_header()` 
for consistency here:

```
for path in source_paths:
with open(path) as source_file:
```



support/mesos-style.py (line 41)


We typically don't use shortened variable names unless it is very clear 
what they are to be used for.  In this case, reading from top to bottom of this 
function, I don't know what `ln` is supposed to signify until I look deeper 
into the function.  We should probably call this variable `line_number` 
instead.  Also, we should probably put spaces around the equal sign as before, 
e.g.:

```
line_number = 1
```



support/mesos-style.py (line 43)


This will not work with `re.match()`. As is, this will only match if the 
non-ascii character is the first character on the line.  Instead, you should 
probably be using `re.search()`.

Also, the variable `m` should probably be renamed `match` and spaces should 
be added around the `=`.



support/mesos-style.py (line 45)


To make things more readable, we typically use python's `.format()` 
function to format strings rather than manually concatenate them together e.g.:

```
"Non ascii character found in {path} "
"(Ln: {line_number}, Pos: {position}): {line}".\
format(
path=path,
line_number=line_number,
position=str(match.start() + 1),
line=line))
```

**Note**: We typically **don't** put spaces around the `=` when assigning 
keyword arguments in parameters.



support/mesos-style.py (lines 145 - 148)


I would probably reorganize this to do the checks first and the linting 
last. Assuming the name `check_encoding()` as mentioned in a previous comment, 
I would probably reorder this as:

```
license_errors = check_license_header(filtered_candidates_set)
encoding_errors = check_encoding(list(filtered_candidates_set))
lint_errors = run_lint(list(filtered_candidates_set))
total_errors = license_errors + encoding_errors + lint_errors
```


- Kevin Klues


On April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report 

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45033]

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 April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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



Thanks for all the input from every one. I updated the review request and 
merged the ascii check with mesos-style.py. I also skipped the .md check for 
now as there is another ticket MESOS-5077 for that. Let me know if there are 
other issues.

- Yong Tang


On April 2, 2016, 1:41 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated April 2, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
> Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi 
> Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add addition check in mesos-style.pl
> for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
> files and report error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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

(Updated April 2, 2016, 1:41 a.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Bernd 
Mathiske, haosdent huang, Kevin Klues, Neil Conway, Vinod Kone, and Deshi Xiao.


Changes
---

Merge ascii check into mesos-style.py, leave .md check to MESOS-5077 according 
to feedbacks.


Bugs: MESOS-4033
https://issues.apache.org/jira/browse/MESOS-4033


Repository: mesos


Description (updated)
---

This review request tries to add addition check in mesos-style.pl
for checking non-ascii characters. It scans .cpp, .hpp, .cc, .h
files and report error if non-ascii characters exists.

As part of this review request, two non-ascii characters are identified
in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
accordingly.

Note: .md scan is skipped based on feedback from review request.


Diffs (updated)
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/mesos-style.py 13616065ebe07ca401b385716d9b723f65bb2162 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Benjamin Bannier]

> On April 1, 2016, 6:53 p.m., Kevin Klues wrote:
> > As Neil said, I would also recommend *not* excluding the docs files. In the 
> > rare case that we need non-ascii cahracters in the docs, we should use HTML 
> > character entities.
> > 
> > I would also not create a whole new python script for this.  It should be a 
> > simple addition to the existing mesos-style.py script.

Shouldn't we switch to a markdown converter which can automatically translate 
unicode to HTML entities (kramdown appears to be capable of this) instead of 
writing even  more markdown documentation tailored to just one output format 
(here: HTML)? If we'd stick to clean markdown we might even be able to generate 
documentation in different formats in the future (e.g., man pages or info docs 
with Pandoc come to mind).

I filed MESOS-5077 so we can independently of the issues discussed here verify 
the validity of generated HTML (something worth tackling on its own right), but 
I think a fix there might well be able to address your and Neil's concerns 
expressed here.


- Benjamin


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


On March 23, 2016, 3:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 23, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Deshi Xiao]

> On 三月 23, 2016, 8:21 a.m., Deshi Xiao wrote:
> > can we ignore .md scan? the .md possible container non-ascii characters.
> 
> Neil Conway wrote:
> Are non-ASCII characters in .md files actually required? We should be 
> able to use HTML character entities instead (e.g., ``) instead.
> 
> Deshi Xiao wrote:
> Neil, see this case:
> 
> quoted from haosdent's comments:
>  If disable no-ascii characters, we could not use emoji and some 
> languages which contains no-ascii characters. For the powered-by-mesos.md and 
> user-groups.html.md, this hard limit seems inconvenient.
> 
> haosdent huang wrote:
> Still could convert them to unicode code and represented by html 
> character entities. Just looks confuse and not straightforward.
> 
> Deshi Xiao wrote:
> same here same concerns
> 
> Neil Conway wrote:
> Personally I would opt for allowing only ASCII characters in .md files 
> and using character entities for the (rare) cases where non-ASCII characters 
> are required.

if we can strict the md only contain ASCII characters, the opt for is fine. so 
here we need a shepherd to help us guideline the way.


- Deshi


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


On 三月 23, 2016, 2:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated 三月 23, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Neil Conway]

> On March 23, 2016, 8:21 a.m., Deshi Xiao wrote:
> > can we ignore .md scan? the .md possible container non-ascii characters.
> 
> Neil Conway wrote:
> Are non-ASCII characters in .md files actually required? We should be 
> able to use HTML character entities instead (e.g., ``) instead.
> 
> Deshi Xiao wrote:
> Neil, see this case:
> 
> quoted from haosdent's comments:
>  If disable no-ascii characters, we could not use emoji and some 
> languages which contains no-ascii characters. For the powered-by-mesos.md and 
> user-groups.html.md, this hard limit seems inconvenient.
> 
> haosdent huang wrote:
> Still could convert them to unicode code and represented by html 
> character entities. Just looks confuse and not straightforward.
> 
> Deshi Xiao wrote:
> same here same concerns

Personally I would opt for allowing only ASCII characters in .md files and 
using character entities for the (rare) cases where non-ASCII characters are 
required.


- Neil


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


On March 23, 2016, 2:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 23, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Neil Conway]

> On March 23, 2016, 8:21 a.m., Deshi Xiao wrote:
> > can we ignore .md scan? the .md possible container non-ascii characters.

Are non-ASCII characters in .md files actually required? We should be able to 
use HTML character entities instead (e.g., ``) instead.


- Neil


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


On March 23, 2016, 2:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 23, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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



Skipped the .md scan based on feedback from haosdent and Deshi.

- Yong Tang


On March 23, 2016, 2:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 23, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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

(Updated March 23, 2016, 2:46 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent huang, 
Neil Conway, and Deshi Xiao.


Changes
---

Skipped .md scan based on feedback from haosdent and Deshi.


Bugs: MESOS-4033
https://issues.apache.org/jira/browse/MESOS-4033


Repository: mesos


Description (updated)
---

This review request tries to add a commit hook for checking non-ascii
characters. It scans .cpp, .hpp, .cc, .h files and report
error if non-ascii characters exists.

As part of this review request, two non-ascii characters are identified
in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
accordingly.

Note: .md scan is skipped based on feedback from review request.


Diffs (updated)
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
  support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
  support/non-ascii.py PRE-CREATION 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Deshi Xiao]

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



can we ignore .md scan? the .md possible container non-ascii characters.

- Deshi Xiao


On 三月 22, 2016, 11:49 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated 三月 22, 2016, 11:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[haosdent huang]

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




docs/versioning.md (line 85)


Any reason we need change here?



support/non-ascii.py (line 21)


Seems we have to update this when we update third part dependencies every 
time? For example, if we update protobuf to 2.6, this rule need update as well. 
Is it possible to avoid this?


- haosdent huang


On March 18, 2016, 2:48 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 18, 2016, 2:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45033]

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 March 19, 2016, 6:26 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 19, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On March 18, 2016, 7:03 p.m., haosdent huang wrote:
> > support/non-ascii.py, line 21
> > 
> >
> > Seems we have to update this when we update third part dependencies 
> > every time? For example, if we update protobuf to 2.6, this rule need 
> > update as well. Is it possible to avoid this?

Hi haosdent, I take a look at this issue. The exclude_file pattern was from 
mesos-style.py. Not knowing enough about the source tree history of mesos 
though it turns out that 3rd party files are actually placed inside the build 
directory and is filtered out anyway. So those version specific patterns are 
not needed any more. I just removed those unneeded patterns.


- Yong


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


On March 19, 2016, 6:26 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 19, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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

(Updated March 19, 2016, 6:26 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
huang.


Changes
---

Remove unneeded version filtering as is suggested by haosdent.


Bugs: MESOS-4033
https://issues.apache.org/jira/browse/MESOS-4033


Repository: mesos


Description
---

This review request tries to add a commit hook for checking non-ascii
characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
error if non-ascii characters exists.

As part of this review request, two non-ascii characters are identified
in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
accordingly.


Diffs (updated)
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
  support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
  support/non-ascii.py PRE-CREATION 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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

(Updated March 19, 2016, 6:08 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
huang.


Changes
---

Address issues suggested by haosdent.


Bugs: MESOS-4033
https://issues.apache.org/jira/browse/MESOS-4033


Repository: mesos


Description
---

This review request tries to add a commit hook for checking non-ascii
characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
error if non-ascii characters exists.

As part of this review request, two non-ascii characters are identified
in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
accordingly.


Diffs (updated)
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
  support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
  support/non-ascii.py PRE-CREATION 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On March 18, 2016, 7:08 p.m., haosdent huang wrote:
> > support/non-ascii.py, line 47
> > 
> >
> > Is it possible to print the match test as well? Sometimes find the 
> > invalid char in a long line is not easy.

I added a position ("Pos") in addition to the line number. Printing out the 
exact character is fairly complicated. We can certainly print out the hex value 
like 0x0A, 0x80, etc. but that will not be too helpful. On the other hand, 
since some of the characters are just invalide characters (not necessarily 
unicode strings) so it may not be printable anyway. I think combining line 
number with the position should be OK to figure out the place to make 
adjustment.


- Yong


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


On March 18, 2016, 11:11 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 18, 2016, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

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

Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


Bugs: MESOS-4033
https://issues.apache.org/jira/browse/MESOS-4033


Repository: mesos


Description
---

This review request tries to add a commit hook for checking non-ascii
characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
error if non-ascii characters exists.

As part of this review request, two non-ascii characters are identified
in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
accordingly.


Diffs
-

  docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
  support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
  support/non-ascii.py PRE-CREATION 

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


Testing
---

Tested manually and found two non ascii characters in docs/versioning.md (fixed 
as part of this review request).


Thanks,

Yong Tang

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45033]

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 March 18, 2016, 11:11 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 18, 2016, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On March 18, 2016, 7:03 p.m., haosdent huang wrote:
> > docs/versioning.md, line 85
> > 
> >
> > Any reason we need change here?

Hi haosdent, the left is one unicode (U+2026) of "Horizontal Ellipsis" (three 
dots). Inside the md file you will see it is 0xE2,0x80,0xA6 (UTF-8) if you use 
binary mode in vi.  The right are three ascii characters of dot (0xA0).


- Yong


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


On March 18, 2016, 2:48 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 18, 2016, 2:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[haosdent huang]

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




support/non-ascii.py (line 47)


Is it possible to print the match test as well? Sometimes find the invalid 
char in a long line is not easy.


- haosdent huang


On March 18, 2016, 2:48 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 18, 2016, 2:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[haosdent huang]

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




support/hooks/pre-commit (line 38)


Do we need add it to `support/hooks/post-rewrite` as well? Because we could 
rewrite commit message when `amend` or `rebase`.


- haosdent huang


On March 18, 2016, 2:48 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 18, 2016, 2:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[haosdent huang]

> On 三月 18, 2016, 7:03 p.m., haosdent huang wrote:
> > docs/versioning.md, line 85
> > 
> >
> > Any reason we need change here?
> 
> Yong Tang wrote:
> Hi haosdent, the left is one unicode (U+2026) of "Horizontal Ellipsis" 
> (three dots). Inside the md file you will see it is 0xE2,0x80,0xA6 (UTF-8) if 
> you use binary mode in vi.  The right are three ascii characters of dot 
> (0xA0).
> 
> Yong Tang wrote:
> By the way, thanks for the review and the help. I will address the rest 
> of the issues later.

Thank you for explaination. Got it.


- haosdent


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


On 三月 18, 2016, 11:11 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated 三月 18, 2016, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Yong Tang]

> On March 18, 2016, 7:03 p.m., haosdent huang wrote:
> > docs/versioning.md, line 85
> > 
> >
> > Any reason we need change here?
> 
> Yong Tang wrote:
> Hi haosdent, the left is one unicode (U+2026) of "Horizontal Ellipsis" 
> (three dots). Inside the md file you will see it is 0xE2,0x80,0xA6 (UTF-8) if 
> you use binary mode in vi.  The right are three ascii characters of dot 
> (0xA0).

By the way, thanks for the review and the help. I will address the rest of the 
issues later.


- Yong


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


On March 18, 2016, 2:48 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 18, 2016, 2:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h, and .md files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>