search

google / closure-linter

Automatically exported from code.google.com/p/closure-linter
Apache License 2.0
109 stars 37 forks source link

added new es3 flag for trailing commas #95

Closed manelvf closed 8 years ago

manelvf commented 8 years ago

Adds a -es3 flag that enables again trailing commas errors, equivalent to the jshint one.

rumpeltux commented 8 years ago

Hi, thanks for the patch! I see you’re essentially reverting https://github.com/google/closure-linter/commit/d052761dab2610e29eafdadefb205edb07a286f6 flag-protecting the check. Can you split this into two patches? 1) reverts the commit and 2) adds the flag-protection. We have relatively strict rules on python-style so this will save me from pointing out the indentation issues and similar stuff. Also it’ll be clearer that the code doesn’t break anything because it’s been in use for a long time.

Thanks!

manelvf commented 8 years ago

Hi,

The problem with reverting the patch is that the test has to be still valid, because the default behavior has to be the same as now. That's the reason I added a new test for the new 'es3' flag. Also, the code has changed from since that commit, so quite sure there would be conflicts.

BTW, you are using a 2 char indent, not the flake8 4 char I'm used. I you tell me the linter you use, I can apply the rules myself.

Please, tell me if you still want the 2 commit split.

Thanks

2015-09-10 9:35 GMT+02:00 Hagen Fritsch notifications@github.com:

Hi, thanks for the patch! I see you’re essentially reverting d052761 https://github.com/google/closure-linter/commit/d052761dab2610e29eafdadefb205edb07a286f6 flag-protecting the check. Can you split this into two patches? 1) reverts the commit and 2) adds the flag-protection. We have relatively strict rules on python-style so this will save me from pointing out the indentation issues and similar stuff. Also it’ll be clearer that the code doesn’t break anything because it’s been in use for a long time.

Thanks!

— Reply to this email directly or view it on GitHub https://github.com/google/closure-linter/pull/95#issuecomment-139143867.

Manel Villar Paulsborner Str. 9 10709 Charlottenburg Berlin (Deutschland)

Tel: (+49) (0)162 6574604 skype: manel.villar manelvf@gmail.com

"Ars longa, vita brevis"

rumpeltux commented 8 years ago

Sorry for taking so long to respond. Linter is pylint, style guide is here: https://google-styleguide.googlecode.com/svn/trunk/pyguide.html One patch seems fine then. Please rename the command line flag to --check_trailing_comma or similar

Thanks!

manelvf commented 8 years ago

Thanks for accepting my PR. I changed the flag for your suggestion.

rumpeltux commented 8 years ago

Please fix the two indents, otherwise looks good. Thanks!

manelvf commented 8 years ago

The two indents are fixed. Could you merge it?

rumpeltux commented 8 years ago

Done. Sorry for taking so long.