Bug #1931

mixed tabs/spaces, trailing whitespace, and inconsistent formatting

Added by Ian Taylor over 3 years ago. Updated over 3 years ago.

Status:Closed Start date:02/05/2009
Priority:Low Due date:
Assignee:Ian Taylor % Done:

0%

Category:-
Target version:-
Keywords: Affected Facter version:
Branch:
Votes: 0

Description

Here is a patch which fixes all, or most formatting inconsistencies in facter.

I know it’s a large patch. I tried to make it simple to ensure that I only did whitespace modification. After applying with git, you can do a @git diff -w@ to see the non-whitespace differences. There should only be a few additions/subtractions of newlines.

0001-replaced-tabs-with-spaces-and-made-spacing-consisten.patch (215.2 kB) Ian Taylor, 02/05/2009 02:50 am

0001-more-consistent-indentation-and-alignment.-also-remo.patch (81.3 kB) Ian Taylor, 02/16/2009 07:51 pm

0001-more-consistent-indentation-and-alignment.-also-remo.patch (81.3 kB) Ian Taylor, 02/18/2009 12:25 am

History

Updated by Luke Kanies over 3 years ago

  • Status changed from Unreviewed to Code Insufficient

I like the idea of making the whole project consistent, but… both Puppet and Facter are four-space projects so far, and this patch changes Facter to be two-space.

At the least, I don’t want to make this change without consultation with the list, and, really, I’m not a big fan of 2 space.

What do others think?

Updated by Ian Taylor over 3 years ago

Either way, maybe it’d be a good idea to have a style guide for contributions? I know that I’d feel better about contributing if there were a few guidelines.

Updated by James Turnbull over 3 years ago

All we really have is:

http://reductivelabs.com/trac/puppet/wiki/DevelopmentLifecycle#style

Updated by Jos Backus over 3 years ago

Fwiw, most Ruby code I have seen, including what ships with Ruby itself (stdlib, etc.) uses 2 spaces, not 4.

Updated by Luke Kanies over 3 years ago

josb wrote:

Fwiw, most Ruby code I have seen, including what ships with Ruby itself (stdlib, etc.) uses 2 spaces, not 4.

That’s true, but Puppet is older than the majority of Ruby projects, and the whole two-space trend didn’t start until I already had a ton of code.

Updated by Ian Taylor over 3 years ago

So, should I resubmit with 4 space indentation?

Updated by Luke Kanies over 3 years ago

Yes, please.

Updated by Ian Taylor over 3 years ago

Okay, here is a new patch.

It makes indentation and alignment more consistent (no tabs, and indentation of 4 spaces) and removes trailing whitespace.

Updated by Luke Kanies over 3 years ago

  • Assignee set to Ian Taylor

The patch doesn’t apply cleanly against the current HEAD:

luke@phage $ git am ~/Desktop/0001-more-consistent-indentation-and-alignment.-also-remo.patch 
Applying: more consistent indentation and alignment. also removal of trailing whitespace
/Users/luke/git/facter/.git/rebase-apply/patch:366: trailing whitespace.
 
/Users/luke/git/facter/.git/rebase-apply/patch:1394: trailing whitespace.
    end 
/Users/luke/git/facter/.git/rebase-apply/patch:1471: trailing whitespace.
            end 
error: patch failed: lib/facter.rb:28
error: lib/facter.rb: patch does not apply
Patch failed at 0001.
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
[master ~/git/facter]
luke@phage $

If I knew git better, I could try to fix it, but… I don’t know to even figure out where the problem is, much less how to fix it.

If you can rebase and republish, I’ll get it applied immediately.

Updated by Ian Taylor over 3 years ago

Not sure what happened. Submitting new patch.

Updated by Luke Kanies over 3 years ago

  • Status changed from Code Insufficient to Closed

Pushed as commit:1e8fcaaa689b535ff3d4d8a30bc13841492730bc.

Also available in: Atom PDF