fix whitespace#40
Conversation
jhoblitt
commented
Jul 7, 2015
- expand tabs into spaces
- fix inconsistent indenting
- minor formatting changes to improve "style" consistency
- remove trailing whitespace from end of line
- remove trailing newline from end of file
|
@samrocketman Unfortunately, expanding just the tabs resulted in monster diff that would be a "rebase barrier" on its own. It also broke mixed tab/whitespace indenting on successive lines and each file needed to be manually inspected.. Since I had to eyeball every line, I went ahead and tried to make the coding style a bit more consistent across files. |
|
I generally support consistency in spacing style, but given massive changes in the history across lots of files like this it's probably better not to merge this. Doing so would make blame essentially useless because of the amount of rewriting that's going on here. |
|
Heyas @skottler! In general, I agree but the mixed tabs/spaces in this plugin are pervasive enough that it complicates PR review and make it painful to edit files with an editor configured to highlight whitespace issues. @samrocketman asked for a reformatting PR in #39 (comment). |
|
I don't think blame would be useless. I think someone would use blame and see the whitespace commit. Then use blame again on the commit before whitespace. That would be what I would do. At some point a change like this should be done otherwise it will just remain inconsistently formatted for the lifetime of the project. I'll let others weigh in. |
|
That means that instead of being able to immediately understand the history of a file you have to turn it into a multi-step process, which is less than ideal for not a lot of immediate gain.
The whitespace would progressively get fixed as the project changes over time. Regardless I don't really work on the plugin anymore so if you think it's a good move then go for it. |
|
I was going to spout a lot of philosophy about whitespace, git-blame, git-log, separating formatting from functionality in commits, and so forth. Ultimately, no one wants to hear that. What is relevant, however, is this: |
|
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
|
I have performed a release. I realize it's not really rebase-able due to the amount of white space conflicts. Therefore, I recommend to start over from the latest master. e.g. git fetch origin
git checkout maint/fix-whitespace
git reset --hard origin/master
#make white space fixes again and commit
git push --force origin maint/fix-whitespace |
* expand tabs into spaces * fix inconsistent indenting * minor formatting changes to improve "style" consistency * remove trailing whitespace from end of line * remove trailing newline from end of file
af46ed2 to
02614ad
Compare
|
@samrocketman I've rebased the non-conflicting files and re-mangled files with changes. That was significantly less fun the second time... |
|
Thanks for your time. It was necessary for me to get those other changes out the door for a release. You won't have to do a second rebase because this change is prioritized next. I'll test that this evening and if it all looks good then I'll merge. |
|
Successfully reviewed and tested. Thanks for taking the time to correct the white space. I appreciate it. |
|
\o/ |