On 1/15/2013 7:31 PM, Brad Roberts wrote:
> Correct, and github has no mechanism for it either. So, I'm happy to add this checking to the load balancer, but someone else build the hook and play with it and get it working w/in their own environment first . There's no reason it shouldn't be put in developer repositories and catch the problem BEFORE they get committed locally. The auto tester can be the second layer of defense.

While you're adding the check for CRLF in the autotester, can you please also add a check to reject any .d files with tab characters? (Just for .d files. Don't want to do that for makefiles!)
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

Walter Bright, el 15 de January a las 21:30 me escribiste:
> 
> On 1/15/2013 7:31 PM, Brad Roberts wrote:
> >Correct, and github has no mechanism for it either. So, I'm happy to add this checking to the load balancer, but someone else build the hook and play with it and get it working w/in their own environment first . There's no reason it shouldn't be put in developer repositories and catch the problem BEFORE they get committed locally. The auto tester can be the second layer of defense.
> 
> While you're adding the check for CRLF in the autotester, can you please also add a check to reject any .d files with tab characters? (Just for .d files. Don't want to do that for makefiles!)

While you're at it, you could also add some checks for other common white space errors. The sample pre-commit hook that comes with each repository have this check enabled, you just have to enable that hook by renaming it from pre-commit.sample to just pre-commit. Then you have to adjust the git config core.whitespace to whatever you want to check (there are several options, see git config --help for details). Unfortunately those checks don't include either the CRLF check or the "all spaces" (there is an option to check the opposite, that only tab is used for indentation), but it cover other common errors as trailing white spaces.

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Algún día los libros desterrarán a la radio y el hombre descubrirá el
oculto poder del Amargo Serrano.
	-- Ricardo Vaporeso. El Bolsón, 1909.
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 1/16/2013 1:34 PM, Leandro Lucarella wrote:
> Walter Bright, el 15 de January a las 21:30 me escribiste:
>> On 1/15/2013 7:31 PM, Brad Roberts wrote:
>>> Correct, and github has no mechanism for it either. So, I'm happy
>>> to add this checking to the load balancer, but someone else build
>>> the hook and play with it and get it working w/in their own
>>> environment first . There's no reason it shouldn't be put in
>>> developer repositories and catch the problem BEFORE they get
>>> committed locally. The auto tester can be the second layer of
>>> defense.
>> While you're adding the check for CRLF in the autotester, can you
>> please also add a check to reject any .d files with tab characters?
>> (Just for .d files. Don't want to do that for makefiles!)
> While you're at it, you could also add some checks for other common
> white space errors. The sample pre-commit hook that comes with each
> repository have this check enabled, you just have to enable that hook by
> renaming it from pre-commit.sample to just pre-commit. Then you have to
> adjust the git config core.whitespace to whatever you want to check
> (there are several options, see git config --help for details).
> Unfortunately those checks don't include either the CRLF check or the
> "all spaces" (there is an option to check the opposite, that only tab is
> used for indentation), but it cover other common errors as trailing
> white spaces.


Does this happen on the user's git, or github? Note that we cannot control the user's git.

_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On Jan 16, 2013, at 4:34 PM, Leandro Lucarella <luca@llucax.com.ar> wrote:
> 
> While you're at it, you could also add some checks for other common white space errors. The sample pre-commit hook that comes with each repository have this check enabled, you just have to enable that hook by renaming it from pre-commit.sample to just pre-commit. Then you have to adjust the git config core.whitespace to whatever you want to check (there are several options, see git config --help for details). Unfortunately those checks don't include either the CRLF check or the "all spaces" (there is an option to check the opposite, that only tab is used for indentation), but it cover other common errors as trailing white spaces.

If there's going to be advanced validation, maybe it's better to create a formatting tool similar to gofmt. If passing a source file through it alters the file, it fails. The tool can be shared and could help establish a standard format for D code (or at least Phobos code).

I've never used gofmt, but did find the following link online: http://golangtutorials.blogspot.com/2011/06/formatting-go-code-with-gofmt.html

On 1/16/2013 2:54 PM, Jason House wrote:
>
> If there's going to be advanced validation, maybe it's better to create a formatting tool similar to gofmt. If passing a source file through it alters the file, it fails. The tool can be shared and could help establish a standard format for D code (or at least Phobos code).

That's why I wrote the tools tolf and detab. Unfortunately, people don't use them. Hence having the autotester reject them.

Here they are:

https://github.com/D-Programming-Language/tools

Walter Bright, el 16 de January a las 13:45 me escribiste:
> 
> On 1/16/2013 1:34 PM, Leandro Lucarella wrote:
> >Walter Bright, el 15 de January a las 21:30 me escribiste:
> >>On 1/15/2013 7:31 PM, Brad Roberts wrote:
> >>>Correct, and github has no mechanism for it either. So, I'm happy to add this checking to the load balancer, but someone else build the hook and play with it and get it working w/in their own environment first . There's no reason it shouldn't be put in developer repositories and catch the problem BEFORE they get committed locally. The auto tester can be the second layer of defense.
> >>While you're adding the check for CRLF in the autotester, can you please also add a check to reject any .d files with tab characters? (Just for .d files. Don't want to do that for makefiles!)
> >While you're at it, you could also add some checks for other common white space errors. The sample pre-commit hook that comes with each repository have this check enabled, you just have to enable that hook by renaming it from pre-commit.sample to just pre-commit. Then you have to adjust the git config core.whitespace to whatever you want to check (there are several options, see git config --help for details). Unfortunately those checks don't include either the CRLF check or the "all spaces" (there is an option to check the opposite, that only tab is used for indentation), but it cover other common errors as trailing white spaces.
> 
> Does this happen on the user's git, or github? Note that we cannot control the user's git.

User's git if set up as a hook. But I was talking about putting it as a safety net in the autotester.

BTW, you can put some kind of hooks in github, but using the API: https://help.github.com/articles/post-receive-hooks

But of course you need a web server hosting the hooks to actually work.

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
PROTESTA EN PLAZA DE MAYO: MUSICO SE COSIO LA BOCA
	-- Crónica TV
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals