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

2016-04-03 Thread 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

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

2016-04-03 Thread 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:

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

2016-04-03 Thread 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. > >

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

2016-04-03 Thread 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:

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

2016-04-03 Thread 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. > >

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

2016-04-03 Thread 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)

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

2016-04-03 Thread 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

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

2016-04-03 Thread 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

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

2016-04-03 Thread 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

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

2016-04-03 Thread 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): > > ... > >

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

2016-04-03 Thread 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

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

2016-04-03 Thread 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. -

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

2016-04-03 Thread 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

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

2016-04-03 Thread 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

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

2016-04-03 Thread 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

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

2016-04-01 Thread 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:

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

2016-04-01 Thread 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

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

2016-04-01 Thread 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

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

2016-04-01 Thread 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

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

2016-03-25 Thread 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. > >

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

2016-03-25 Thread 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. >

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

2016-03-24 Thread 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

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

2016-03-23 Thread 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.

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

2016-03-23 Thread 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

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

2016-03-23 Thread 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

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

2016-03-20 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45033/#review124253 --- docs/versioning.md (line 85)

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

2016-03-19 Thread 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:

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

2016-03-19 Thread 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

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

2016-03-19 Thread 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

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

2016-03-19 Thread 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

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

2016-03-19 Thread 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

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

2016-03-19 Thread 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:

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

2016-03-19 Thread 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:

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

2016-03-19 Thread 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).

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

2016-03-19 Thread 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)

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

2016-03-19 Thread 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)

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

2016-03-19 Thread 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

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

2016-03-18 Thread 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