Ruby Style Agreement about sketchup-stl HOT 24 OPEN

That works for me. Are we using the TomDoc style to document methods as well?

from sketchup-stl.

Notes on the style guide:

Never use for, unless you know exactly why. Most of the time iterators should be used instead. for is implemented in terms of each (so you're adding a level of indirection), but with a twist - for doesn't introduce a new scope (unlike each) and variables defined in its block will be visible outside it.

I use for more often that each these days - mainly because it doesn't introduce a new scope. Because it doesn't introduce a new scope the variables inside the loop isn't created and destroyed for every iteration - as oppose to what each does. Creating variables are slow - and when you iterate entity collections where you have thousands to millions of items the performance can be noticed. So in terms of iterating large collections such as entities I do recommend for loops.

from sketchup-stl.

And I just want to highlight this item on the style list:

The and and or keywords are banned. It's just not worth it. Always use && and || instead.

I see that many people actually use and and or - which I think is because it appear to be more natural in terms how you read it - without realizing that they function differently from && and ||. I've always used && and || because they are more predictable IMO.

from sketchup-stl.

What about this one?

http://www.caliban.org/ruby/rubyguide.shtml#style

This is the one we're attempting to use internally.

from sketchup-stl.

Works for me Scott.

from sketchup-stl.

Seems to be quite similar to the other one. (The other one seemed to explain thing better IMO.) Works for me.
Can we convert the importer to 2-space indentation now, please?

from sketchup-stl.

I see Jim removes his feature branches after they've been merged. Is this normal to do? How does that affect the commit history?

from sketchup-stl.

Another workflow question:

I've just completed the Merge coplanar faces feature - which I commited to a feature branch "feature-merge-faces" and made a Pull request.

What do I do if I want to work on some other thing until it's merged? I'd like to fix the path issue and some other small fixes. But how could I commit that? To a new branch based on the one I'd just worked on?

Or jump back to the master branch - without the merge option feature - and create a new parallel one?

from sketchup-stl.

http://scottchacon.com/2011/08/31/github-flow.html

Read this article from the GitHub people - but still not wiser on how to work on multiple fixes while awaiting a Pull Request.

from sketchup-stl.

Note for cleaning up:

Exporter is using quite a few and operators intead of &&.

Example:

if (foo == 'bar') and (biz == 'baz') and (bim == 'bam')

Which is more cleanly written as: (And recommended by all style guides I've seen)

if foo == 'bar' && biz == 'baz' && bim == 'bam'

from sketchup-stl.

Yeah, I'm not sure the best way to handle these things. I just merged your first request, and the "new" pull request comes across nicely vs. the "latest & greatest" that is now in the repo. So I think this is a decent way to work.

from sketchup-stl.

So what do I now do with my branches? Delete them, and pull the master branch from this main repo?

from sketchup-stl.

Yeah, I seems silly that you'd have to keep old branches around. I suspect github will preserve all of our history for us. So delete away and let's see what happens.

from sketchup-stl.

So, what is the preferred format for documenting methods? RDoc? YARD?

I personally prefer the structured and guided manner YARD document methods. RDoc is too freestyling and hippie-like IMO.

from sketchup-stl.

@jimfoltz : what's your workflow inregard to: #11 (comment)

from sketchup-stl.

My workflow is still evolving.

I was trying to use branches to group together smaller, related commits for GitHub pull requests. I am not sure how this is working for others. Maybe since the files here are fairly small, it is easier to just review the entire file rather than review each tiny change.

I feel stuck at the moment because I am not sure what will happen with the current pull requests. I am not sure if I should base new changes on my branch which may or may not be accepted, or if I should always go ack to the current sketchup/aster branch and create parallel branches from there.

from sketchup-stl.

What do you do with branches after they are merged with this repo's master?
Delete? Erase? Merge with your master?

I also feel a bit stuck. You've got some large changes, but I want to wait until it's been reviewed and merged before I continue.

The SketchUppers are in Las Vegas I think, at Trimble Dimension event. Once Scott is back from that we can get your branch merged.

from sketchup-stl.

Btw, what timezone are you on Jim..?

from sketchup-stl.

US Eastern.

I haven't settled on a branch workflow, because I'm still unsure how best to use them on GitHub. But this page[1] and the next has a good explanation of branches. If you want to be able to easily go back to commit, keep the branch (or tag it.)

[1] http://think-like-a-git.net/sections/experimenting-with-git.html

from sketchup-stl.

Hey guys, sorry I disappeared. We've indeed been in Vegas at the Trimble Dimensions conference. I'm reviewing requests now... :)

from sketchup-stl.

Useful and interesting breakdown of some Git concepts: http://think-like-a-git.net/sections/testing-out-merges.html

from sketchup-stl.

Don't think we reached a conclusion regarding what style to follow. Anyone?

from sketchup-stl.

Re: Style Guides
(1) The internal GitHub guide has no weight over other guides. Personally I find it inferior to others, and much of it very subjective to the author.
It's only pro is the amount of reasons given (although I disagree with many of the reasons.)

(2) Most of the guides are written towards System Ruby scripts where there is no danger of clashing with other coder's scripts.
The caliban.org guide is a case in point. They want coders to put include statements before class and module definitions. That is a no-no. That would mix in libraries into the toplevel ObjectSpace!
The clause about wrapping a test in an if conditional that compares the current script path to $0 is another. (In SketchUp embedded Ruby the $0 global is the string "SketchUp".

(3) A shebang is not needed in a embedded Ruby code file, but the new magic # encoding: UTF-8 1st line comment should be in every file.

(4) I hate confusing "always do this, except in these cases", etc. Case in point whitespace. Either you always use it around = and operators, or you don't. The bottom line is Ruby is supposed to be human readable. That means whitespace. Making little exceptions like "not after (" or "not before )" do not make code more readable. It does the opposite. Personally with regard to parenthesis and square brackets (subscripts/indices) whatever makes it more readable. I like to put spaces around the argument, IF the expression is complex, or a constant or long-named variable reference. But if it is a literal number index or a small iterator variable like i, then I will not put spaces around it.

(5) The doc generator may not belong in a style guide. Every guide specifies some other format.
Personally I have not found one I like. I think a method's comments & documentation should go inside itself. I fold up may scripts using Notepad++'s ALT+number (where the number is the nesting level to collapse. SHFT+ALT+number expands that nesting level.
When I want the methods of a class or module collapsed, I meant it! I still do not want to be scrolling through a bunch of commenting. Some of these doc generators insist that the comments go just before the method definition.
I put method notes just below in def line at the same indent. It looks like the comments are hanging from the definition statement. Same for modules and classes.

from sketchup-stl.

We decided on the GitHub Ruby styleguide for the SketchUp GitHub project a good while ago. This issue just wasn't updated. Simply because it's a well known style that most open source involved people is familiar with. And consistency is the biggest value to a style guide.

Need to update the README to reflect this.

from sketchup-stl.