From: Nikunj A Dadhania <nikunj@linux•vnet.ibm.com>
To: Segher Boessenkool <segher@kernel•crashing.org>,
Thomas Huth <thuth@redhat•com>
Cc: aik@ozlabs•ru, linuxppc-dev@ozlabs•org, dvaleev@suse•com
Subject: Re: [PATCH SLOF 4/5] disk-label: add support for booting from GPT FAT partition
Date: Wed, 24 Jun 2015 10:59:53 +0530 [thread overview]
Message-ID: <87zj3p7jxq.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <20150624021013.GE19845@gate.crashing.org>
Segher Boessenkool <segher@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
next prev parent reply other threads:[~2015-06-24 5:30 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-22 7:59 [PATCH SLOF 0/5] GPT fixes/cleanup and LVM support with FAT Nikunj A Dadhania
2015-06-22 7:59 ` [PATCH SLOF 1/5] disk-label: simplify gpt-prep-partition? routine Nikunj A Dadhania
2015-06-23 7:08 ` Thomas Huth
2015-06-22 7:59 ` [PATCH SLOF 2/5] introduce 8-byte LE helpers Nikunj A Dadhania
2015-06-23 7:09 ` Thomas Huth
2015-06-22 7:59 ` [PATCH SLOF 3/5] disk-label: introduce helper to check fat filesystem Nikunj A Dadhania
2015-06-22 19:35 ` Segher Boessenkool
2015-06-23 6:06 ` Nikunj A Dadhania
2015-06-22 7:59 ` [PATCH SLOF 4/5] disk-label: add support for booting from GPT FAT partition Nikunj A Dadhania
2015-06-23 7:34 ` Thomas Huth
2015-06-24 2:10 ` Segher Boessenkool
2015-06-24 5:29 ` Nikunj A Dadhania [this message]
2015-06-24 5:33 ` Nikunj A Dadhania
2015-06-22 7:59 ` [PATCH SLOF 5/5] disk-label: make gpt detection code more robust Nikunj A Dadhania
2015-06-23 7:46 ` Thomas Huth
2015-06-24 5:34 ` Nikunj A Dadhania
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zj3p7jxq.fsf@linux.vnet.ibm.com \
--to=nikunj@linux$(echo .)vnet.ibm.com \
--cc=aik@ozlabs$(echo .)ru \
--cc=dvaleev@suse$(echo .)com \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=segher@kernel$(echo .)crashing.org \
--cc=thuth@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox