Trim trailing white space from the source code files

RESOLVED FIXED

Status

Socorro
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: lonnen, Unassigned)

Tracking

Trunk
Other
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

7 years ago
Both our python and php code stye guides discourage trailing whitespace. As part of our larger code cleanup effort we could script removing trailing spaces from uncompressed files in version control. This would knock out more than half of our pep 8 violations alone. Following this change all new code should be trailing white space free. We could script enforcement of this, even.
(Reporter)

Updated

7 years ago
Blocks: 664759
Hrm I see jar and other files in this, which you probably don't want to modify in this way :)

Could you rerun this only against text files? Probably just *.php and *.py would be enough - if you want to do all text files (I assume you are scripting this), the unix "file" command has pretty good heuristics for checking for text files (looks for magic headers, unprintable chars etc)
(Reporter)

Comment 3

7 years ago
Removed the modifications to the binary stuff. 

https://github.com/mozilla/socorro/pull/29
(In reply to Chris Lonnen :lonnen from comment #3)
> Removed the modifications to the binary stuff. 
> 
> https://github.com/mozilla/socorro/pull/29

I think this is fine, the only thing I can think of is - would this hurt any multi-line strings like:
https://github.com/Lonnen/socorro/commit/691aa9978807be1ff98f80e233c877d8bfc9926a#L237R73

I don't *think* it should because SQL understands line breaks, should be fine. I'll merge and we should test that.

The only other thing is that doing this on schema dumps (or any other generated file) is probably futile since it'll keep coming back.

Also, now that you've done this work, can we put a check for Jenkins to run to make sure it doesn't regress? Would you mind filing a bug for that (assuming you agree)?

Running a pep8 checker should help, but we might consider just running a generic one across all text files (perhaps with a blacklist of files like the pg schema).
(Reporter)

Comment 5

7 years ago
(In reply to Robert Helmer [:rhelmer] from comment #4)
> I think this is fine, the only thing I can think of is - would this hurt any
> multi-line strings like:
> https://github.com/Lonnen/socorro/commit/
> 691aa9978807be1ff98f80e233c877d8bfc9926a#L237R73

I looked for multiline strings when I proofed the changes and I didn't catch any that would be problematic. The sql strings should be fine. I was able to run things from my branch without a fuss, but I hardly went through all the pages.

> The only other thing is that doing this on schema dumps (or any other
> generated file) is probably futile since it'll keep coming back.

White list it in our checker (linter?) or implement pre-commit hooks that trim for us. The latter is more difficult and adds complexity.
 
> Also, now that you've done this work, can we put a check for Jenkins to run
> to make sure it doesn't regress? Would you mind filing a bug for that
> (assuming you agree)?

Filed: https://bugzilla.mozilla.org/show_bug.cgi?id=686829

> Running a pep8 checker should help, but we might consider just running a
> generic one across all text files (perhaps with a blacklist of files like
> the pg schema).

Other projects use https://github.com/jbalogh/check, which will run pyflakes, PEP8, and white space checking. We're pretty far from PEP 8 compliance, though. We also might need to fork it to add white listing. Not sure if it would be better to use this now and put up with the warnings or to write a few lines of bash to check for white space and add the python stuff when it's ready... any more discussion in that line of thinking should probably go in the aforelinked bug, though.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Component: Socorro → General
Product: Webtools → Socorro
You need to log in before you can comment on or make changes to this bug.