Ticket/1.6.x/11695 facter comment

[ssvarma]

All fact commenting complete in facter/lib/facter. Thanks Daniel for your feedback.

[slippycheeze]

We discourage this sort of pointless header; it either tells you nothing - because you know what file you opened - or wrong, because it doesn't match the file.

[slippycheeze]

Other than those specific issues, this doesn't automatically merge. Can you also rebase it against master, so we can merge cleanly.

[slippycheeze]

This includes a pile of unrelated changes; can you resubmit a smaller, clean pull request please.

[ssvarma]

Updated and cleaned!

[slippycheeze]

There isn't much value in a banner announcing the current name of the file.

[ssvarma]

Earlier pull request #145 is closed (#145) for this same ticket. This one is the current clean pull request.

[kbarber]

I'm going to echo Daniel's sentiments here ... but this pull request has sat idle for a bit so I thought I'd give it a kick.

Your indentation is eratic - choose either 2, 3 or 4 (I'd recommend either 2 or 4) spaces for indent. Otherwise you make us poor pedants heads explode :-). The point here is your doing an en-masse cleanup of comments - so we want to set a consistent style for others to follow. WIth erratic indentation we don't really achieve that :-).

Don't remove the comment from the real ruby classes/modules - these are used to generate rdoc comments in the API documentation. If anything clean them up ... but don't remove them - they are important. Check this out:

http://rubydoc.info/github/puppetlabs/facter/

Now if you look at say 'IP' you'll see the comment appear in the description. Some of the modules aren't right though - and its probably because the comment appears before the require line (and there is extra-whitespace). If you can fix the comments so they look like ip.rb (comment/description pushed hard up against the 'module' or 'class' block) then it makes the API docs do the right thing.

You can test the format by running 'rdoc' in the facter directory, and pointing a browser at the html dir.

[ssvarma]

Will take care of the indentations Ken. :-) Not going to remove any comments from classes/modules, had just removed the few I had added in the facter/util area as explained above. I will put them back though if that is needed.

Thanks to both of you for such detailed feedback. Appreciate it much.

[ssvarma]

all updated and ready to be merged. Let me know if there is anything else. Thanks again!

[kbarber]

@ssvarma you seemed to have checked in the doc/ directory into the repo. This normally doesn't get committed - can you git rm the directory and then apply another commit? Thanks.

If you like - add the 'doc/' entry to .gitignore after the 'git rm doc/' so it won't keep bothering you when you do a 'git status' :-).

I'll need you to fix that before I can review this - github is having a heart-attack trying to display all the doc/ directory in the diff tab :-).

[ssvarma]

Oops, have taken doc out. It should be all good now! Sorry it took so long, was in Puppet training last week.