Hi everybody,
It's kind of silly when we commit php syntax errors to SVN (I've done it too). Of course we should all test our code before committing, but sometimes we don't--especially when it's a one line change and there's No Way It Could Break.
To help us stop making these silly mistakes (and to avoid the inevitable complaint and followup), I've added a pre-commit hook to SVN.
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
Let me know if you have any problems with it.
-Chad
On Tue, Nov 30, 2010 at 10:04 AM, Chad innocentkiller@gmail.com wrote:
Hi everybody,
It's kind of silly when we commit php syntax errors to SVN (I've done it too). Of course we should all test our code before committing, but sometimes we don't--especially when it's a one line change and there's No Way It Could Break.
To help us stop making these silly mistakes (and to avoid the inevitable complaint and followup), I've added a pre-commit hook to SVN.
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
Let me know if you have any problems with it.
-Chad
Disabled temporarily, I'm hitting issues when doing a svn del.
-Chad
On Tue, Nov 30, 2010 at 10:19 AM, Chad innocentkiller@gmail.com wrote:
On Tue, Nov 30, 2010 at 10:04 AM, Chad innocentkiller@gmail.com wrote:
Hi everybody,
It's kind of silly when we commit php syntax errors to SVN (I've done it too). Of course we should all test our code before committing, but sometimes we don't--especially when it's a one line change and there's No Way It Could Break.
To help us stop making these silly mistakes (and to avoid the inevitable complaint and followup), I've added a pre-commit hook to SVN.
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
Let me know if you have any problems with it.
-Chad
Disabled temporarily, I'm hitting issues when doing a svn del.
-Chad
Syntax errors in the pre-commit file.
Oh the irony.
-Chad
On Tue, Nov 30, 2010 at 10:19 AM, Chad innocentkiller@gmail.com wrote:
On Tue, Nov 30, 2010 at 10:04 AM, Chad innocentkiller@gmail.com wrote:
Hi everybody,
It's kind of silly when we commit php syntax errors to SVN (I've done it too). Of course we should all test our code before committing, but sometimes we don't--especially when it's a one line change and there's No Way It Could Break.
To help us stop making these silly mistakes (and to avoid the inevitable complaint and followup), I've added a pre-commit hook to SVN.
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
Let me know if you have any problems with it.
-Chad
Disabled temporarily, I'm hitting issues when doing a svn del.
-Chad
Syntax errors in the pre-commit file.
Oh the irony.
-Chad
if/when this is enabled. Does this require anything from the commiters ? Do I need to install something or run a command in addition to or instead of 'svn commit -m "" ' ?
Sounds nice as an additional check :)
-- Krinkle
With my limited svn knowledge I'd say no. Postcommit hooks are a part of svn itself, IIRC
I could also be wrong.
-X!
On Nov 30, 2010, at 10:34 AM, Krinkle krinklemail@gmail.com wrote:
On Tue, Nov 30, 2010 at 10:19 AM, Chad innocentkiller@gmail.com wrote:
On Tue, Nov 30, 2010 at 10:04 AM, Chad innocentkiller@gmail.com wrote:
Hi everybody,
It's kind of silly when we commit php syntax errors to SVN (I've done it too). Of course we should all test our code before committing, but sometimes we don't--especially when it's a one line change and there's No Way It Could Break.
To help us stop making these silly mistakes (and to avoid the inevitable complaint and followup), I've added a pre-commit hook to SVN.
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
Let me know if you have any problems with it.
-Chad
Disabled temporarily, I'm hitting issues when doing a svn del.
-Chad
Syntax errors in the pre-commit file.
Oh the irony.
-Chad
if/when this is enabled. Does this require anything from the commiters ? Do I need to install something or run a command in addition to or instead of 'svn commit -m "" ' ?
Sounds nice as an additional check :)
-- Krinkle
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Tue, Nov 30, 2010 at 10:04 AM, Chad innocentkiller@gmail.com wrote:
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
This assumes that all .inc files are actually PHP. Probably they are right now, but I can foresee this potentially breaking years down the line and confusing someone. Maybe for .inc files, you should check if they start with "<?php" before trying php -l.
On Tue, Nov 30, 2010 at 11:55 AM, Aryeh Gregor Simetrical+wikilist@gmail.com wrote:
This assumes that all .inc files are actually PHP. Probably they are right now, but I can foresee this potentially breaking years down the line and confusing someone. Maybe for .inc files, you should check if they start with "<?php" before trying php -l.
That's one option. Another would be to stop using .inc files ;-)
-Chad
"Aryeh Gregor" Simetrical+wikilist@gmail.com wrote in message news:AANLkTi=sGDvuc7f4r_USFYA7E=ceCfSe7BY-TxCn3MS2@mail.gmail.com...
On Tue, Nov 30, 2010 at 10:04 AM, Chad innocentkiller@gmail.com wrote:
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
This assumes that all .inc files are actually PHP. Probably they are right now, but I can foresee this potentially breaking years down the line and confusing someone. Maybe for .inc files, you should check if they start with "<?php" before trying php -l.
Surely if there is no opening <?php tag then there is no parsing todo, and therefore the file will always pass validation.
You may need to set short_open_tag to false to enforce this, I guess (it's good practice, anyhow, so that'd be no bad thing).
- Mark Clements (HappyDog)
On Tue, Nov 30, 2010 at 12:22 PM, Mark Clements (HappyDog) gmane@kennel17.co.uk wrote:
You may need to set short_open_tag to false to enforce this, I guess (it's good practice, anyhow, so that'd be no bad thing).
I'm inclined to check with short open tags off. I know the Zend style guide recommends having against them (ours as well, I think...).
Enforcing that style with a pre-commit hook is kind of nice :p
-Chad
On Tue, Nov 30, 2010 at 12:22 PM, Mark Clements (HappyDog) gmane@kennel17.co.uk wrote:
Surely if there is no opening <?php tag then there is no parsing todo, and therefore the file will always pass validation.
Good point. I forgot how crazy PHP is there for a second. :) A non-PHP file in our repo that contains <?php and ends with ".inc" seems implausible enough to ignore.
Chad,
You are my hero.
Thank you so much for doing this!
Arthur
On 11/30/2010 07:04 AM, Chad wrote:
Hi everybody,
It's kind of silly when we commit php syntax errors to SVN (I've done it too). Of course we should all test our code before committing, but sometimes we don't--especially when it's a one line change and there's No Way It Could Break.
To help us stop making these silly mistakes (and to avoid the inevitable complaint and followup), I've added a pre-commit hook to SVN.
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing. You should get a fun error message on your local console if you commit bad code :)
Let me know if you have any problems with it.
-Chad
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Tue, Nov 30, 2010 at 5:27 PM, Ashar Voultoiz hashar+wmf@free.fr wrote:
On 30/11/10 16:04, Chad wrote:
All changed/added files ending in .inc/.php/.php5 are now checked with php -l prior to the transaction completing.
One step toward quality! Thanks Chad. Is the next one "running parser tests" ?
We had parser tests running in code review on each commit, but it was sluggish so we chucked the feature. This is what ci.tesla does. We just need to fine tune it some more and make it run after every commit.
-Chad
wikitech-l@lists.wikimedia.org