some codesniffer niceups

[jbrekle]

Hi,

i just tested the cs stuff, and i really like it!

I had already installed the CodeSniffer pear library, and cs-install says it fails, because of that, but actually succeeds. maybe you ignore that error there. (should also occur when cs-uninstalling and then cs-install again - because the uninstall doesnt uninstalls the pear stuff)

and if i add a var_dump or print_r (and also exit and echo), i would like cs to complain about that. is there a simple way to disable cs for one(current) commit? for example: i fix all cs errors that i can, but some remain (for example a print_r to the log).

@seebi also suggested that you could make something that prevents us from commiting to the master branch.

what do you think?

regards,
Jonas

[seebi]

yes please!

maybe we have different level here:
level 0:

• only hardcore errors do block the commit, including trying to commit into the master branch
• this is established with "make install" so its default
  level 1:
• our suggested level of quality checks, which is more accurate

btw: does make cs-install works on Darwin as well @pfrischmuth, maybe you can test that?

[seebi]

sorry, wrong button ... :)

[seebi]

additional requests:

• cs-install should test for root (to avoid errors)
• 2x cs-install crashes
• bad words: var_dump, exit; die, _POST, _GET, _REQUEST
• keine tabs
• no whitespaces at the end of the line

[seebi]

after working a little bit with the pre_commit hook, I would like to change that to pre_push if this exist:
the reason: implementing a feature and fixing the code quality should be different commits and I can't commit the feature first ...

what do you think about that?

[seebi]

@Lars: what about this issue? These additional requirements are important:

• cs-install should test for root (to avoid errors)
• 2x cs-install crashes
• test for bad words: var_dump, exit; die, _POST, _GET, _REQUEST
• test for tabs
• test for whitespaces at the end of the line

[seebi]

can we drop Internal.NoCodeFound errors on "make cs-check"

these errors appear on Makefiles, N3 files, ...

[Lars]

Seebi, the opening user was @larseidam not myself.

Thanks

[seebi]

@Lars: thanks ... and sorry :)

[larseidam]

@ALL

now there is a CodeSniffer Update on commit 714ce22

Attention: some Makefile commands have changed (look at 'make help')

fixed issue:

• cs-install should test for root (to avoid errors)
• 2x cs-install crashes
• test for bad words: var_dump, exit; die, _POST, _GET, _REQUEST (also print_r and error_log)
• test for tabs
• test for whitespaces at the end of the line
• install CS on submodules/extensions
• run CS with specific sniffs

open issue:

• wikipage for sniff explanation
• CS integration for Vim
• forbid the committing in the master branch
• install CS on all submodules/extensions

@seebi

What do you mean with this??

can we drop Internal.NoCodeFound errors on "make cs-check"
these errors appear on Makefiles, N3 files, ...

Do you want a sniff that shows files they have no php code, files they are empty or something else??
Maybe we can talk about it on our meeting today.

[seebi]

lars, looks great!

regarding your question: I just dont want an error if the sniffer founds a file which is not php

one solution: the sniffer looks only for php and phtml files
another solution: it looks in every file but do not complain about files without php code

[larseidam]

all issues in this thread are done or rather in a other Issus-thread