The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #1931

mixed tabs/spaces, trailing whitespace, and inconsistent formatting

Added by Ian Taylor almost 6 years ago. Updated almost 6 years ago.

Status:ClosedStart date:02/05/2009
Priority:LowDue date:
Assignee:Ian Taylor% Done:

0%

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

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


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 Magnifier (215 KB) Ian Taylor, 02/05/2009 02:50 am

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

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

History

#1 Updated by Luke Kanies almost 6 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?

#2 Updated by Ian Taylor almost 6 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.

#3 Updated by James Turnbull almost 6 years ago

All we really have is:

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

#4 Updated by Jos Backus almost 6 years ago

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

#5 Updated by Luke Kanies almost 6 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.

#6 Updated by Ian Taylor almost 6 years ago

So, should I resubmit with 4 space indentation?

#7 Updated by Luke Kanies almost 6 years ago

Yes, please.

#8 Updated by Ian Taylor almost 6 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.

#9 Updated by Luke Kanies almost 6 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.

#10 Updated by Ian Taylor almost 6 years ago

Not sure what happened. Submitting new patch.

#11 Updated by Luke Kanies almost 6 years ago

  • Status changed from Code Insufficient to Closed

Pushed as commit:1e8fcaaa689b535ff3d4d8a30bc13841492730bc.

Also available in: Atom PDF