[PATCH SLOF 4/5] disk-label: add support for booting from GPT FAT partition

[Nikunj A Dadhania]

Segher Boessenkool <segher at kernel.crashing.org> writes:

> On Tue, Jun 23, 2015 at 09:34:44AM +0200, Thomas Huth wrote:
>> > +: load-from-gpt-partition ( [ addr ] -- size | TRUE )
>> 
>> What do you mean with addr in square brackets? Is it optional?
>
> And "size | TRUE"?  The code even returns "false" instead, which
> usually is a valid size (0).  Just always return a flag?  Or maybe
> you mean something like ( -- false | size true ) .  Not going to
> read the code, I cannot keep track of the stack, bringing us to...
>
>
>> Hmm, I wonder whether we need a proper coding conventions spec for
>> writing Forth code ... (at least about the indentation depths ...) ;-)
>
> "Write readable code.  That means in part, do not write long definitions
> (longer than a few lines)."

I ended up here by combining two similar looking words as they were
doing too many similar stuff.

But I guess it ended up being pretty big. I will break it up into
smaller units and resend this patch.

>
> There, all coding conventions you'll ever need :-)
>
>
> Almost all short definitions (with good names!) are easily readable
> (with a little effort if the subject matter is tricky).  No longer
> definitions are ever readable (well, there are exceptions; not many).
>
> Don't get hung up on "how many spaces should I indent"...  Since your
> words are short, you won't have more than two levels of indent anyway :-)
>
> Adding extra spacing to group things is also very helpful.
>
> Minor things...  Most words want a stack comment.  If you need stack
> comments inside a definition, it is too complex.  If there is any
> significant amount of stack juggling, the word is too complex.  If
> the word would be too complex, you need to factor it.  If you cannot
> easily split off factors, your solution is too complex.  If it is
> hard to think of good names for the factors, that is simply because
> naming things is the hardest part of programming (but see also the
> previous point).
>
> You also want short words that do one little thing because you _do_
> test your code.

Regards
Nikunj