All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind@infradead.org>
To: Theodore Tso <tytso@mit.edu>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	Frank Haverkamp <haver@vnet.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	David Woodhouse <dwmw2@infradead.org>,
	Josh Boyer <jwboyer@linux.vnet.ibm.com>
Subject: Re: [PATCH 00/44 take 2] [UBI] Unsorted Block Images
Date: Mon, 19 Feb 2007 19:07:46 +0200	[thread overview]
Message-ID: <1171904866.14817.36.camel@sauron> (raw)
In-Reply-To: <20070219143321.GE25490@thunk.org>

On Mon, 2007-02-19 at 09:33 -0500, Theodore Tso wrote:
> It made it much, much, MUCH harder to review.  Especially given that
> the documentation was separated from the implementation.  As I looked
> at the implementation, there was no way to look and what it was
> supposed to do without flipping back to a previous e-mail message and
> losing my place.

I will send Build stuff as the last patch next time, thanks, point
taken.

I just used different concept: one looks at declaration and the overall
picture becomes clear because _there is_ documentation. One does not
look at the implementation to grasp picture on surface.

But your point is fair. I assume _programmers_ look in .c first. Users
may always generate a pdf. I will do what you advise.

May be a good compromise would be to have just brief comments at
headers, and full specification at .c. I will think about this, thanks.

> > This reflects the way of my thinking. I see UBI as a set of units with
> > defined interfaces. So I even physically split the interface description
> > into files. I still think it is easier to grasp the architecture this
> > way.
> 
> Speaking as someone who was coming into it cold, it actually made it
> far more difficult.  Your units were too small, so that meant the
> number of interfaces that were created as a result were huge!  (Around
> 20 _sets_ of interfaces, all of which had to be comprehended for what
> should have been a relatively simple set of functionality!)

Why not? Some stuff may probably be merged. _Specific_ advices are
welcome.

> And when you create that many interfaces, it adds inertia to changing
> the interfaces later on, because it's sometimes not clear how many
> users of the interface there really are.  My general rule of thumb is
> that if an interface only has one user, then it may be a good idea to
> combine it with the user of that interface, and then make the
> functions involved be a static, so that it becomes clear the only user
> of that functoin is within that one file.  You can take this too far,
> and to extremes it doesn't work all that well, but the UBI layer has
> gone waaaaaay off the deep end in terms of functional decomposition.

Well... I do not want any flame on this topic. It is about taste,
trade-offs, compromises. It is difficult to provide _objective_ and
killing arguments here. But I will think on this, point taken, thanks.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)


  reply	other threads:[~2007-02-19 17:51 UTC|newest]

Thread overview: 129+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-17 16:54 [PATCH 00/44 take 2] [UBI] Unsorted Block Images Artem Bityutskiy
2007-02-17 16:54 ` [PATCH 01/44 take 2] [UBI] Linux build integration Artem Bityutskiy
2007-02-17 16:54 ` [PATCH 02/44 take 2] [UBI] on-flash data structures header Artem Bityutskiy
2007-02-17 16:54 ` [PATCH 03/44 take 2] [UBI] user-space API header Artem Bityutskiy
2007-02-17 21:27   ` Arnd Bergmann
2007-02-20 13:07     ` Artem Bityutskiy
2007-02-20 13:17       ` Arnd Bergmann
2007-02-17 16:54 ` [PATCH 04/44 take 2] [UBI] kernel-spce " Artem Bityutskiy
2007-02-18  1:32   ` Greg KH
2007-02-18  2:08     ` Josh Boyer
2007-02-26 12:12     ` Artem Bityutskiy
2007-02-17 16:54 ` [PATCH 05/44 take 2] [UBI] internal common header Artem Bityutskiy
2007-02-17 21:05   ` Arnd Bergmann
2007-02-19 11:16     ` Artem Bityutskiy
2007-02-19 10:54   ` Christoph Hellwig
2007-02-19 12:38     ` Josh Boyer
2007-02-20 13:05     ` Artem Bityutskiy
2007-02-20 14:55       ` Theodore Tso
2007-02-20 15:15         ` David Woodhouse
2007-02-20 15:22           ` Theodore Tso
2007-02-20 15:33             ` David Woodhouse
2007-02-20 16:12               ` Theodore Tso
2007-02-20 16:47                 ` David Woodhouse
2007-02-25 10:42               ` Pavel Machek
2007-02-20 15:24           ` Artem Bityutskiy
2007-02-25  5:45             ` Christoph Hellwig
2007-02-26 10:28               ` Artem Bityutskiy
2007-02-25  5:43           ` Christoph Hellwig
2007-02-25  6:04             ` David Woodhouse
2007-02-20 15:21         ` Artem Bityutskiy
2007-02-25  5:46           ` Christoph Hellwig
2007-02-20 15:25         ` Artem Bityutskiy
2007-02-25  5:50       ` Christoph Hellwig
2007-02-25 11:55         ` Theodore Tso
2007-02-26 10:09         ` Artem Bityutskiy
2007-02-17 16:54 ` [PATCH 06/44 take 2] [UBI] startup code Artem Bityutskiy
2007-02-19 10:59   ` Christoph Hellwig
2007-02-20 13:00     ` Artem Bityutskiy
2007-02-23 11:03       ` Artem Bityutskiy
2007-02-25  5:58       ` Christoph Hellwig
2007-02-25 22:03         ` Rusty Russell
2007-03-05 13:28           ` Frank Haverkamp
2007-02-26 11:54         ` Artem Bityutskiy
2007-05-17 14:44         ` Christoph Hellwig
2007-05-17 15:06           ` Artem Bityutskiy
2007-02-17 16:54 ` [PATCH 07/44 take 2] [UBI] misc unit header Artem Bityutskiy
2007-02-17 22:59   ` Theodore Tso
2007-02-19 11:00     ` Christoph Hellwig
2007-02-20 12:56       ` Artem Bityutskiy
2007-02-19 11:13     ` Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 08/44 take 2] [UBI] misc unit implementation Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 09/44 take 2] [UBI] debug unit header Artem Bityutskiy
2007-02-17 21:18   ` Arnd Bergmann
2007-02-19 11:00     ` Christoph Hellwig
2007-02-19 12:33     ` Artem Bityutskiy
2007-02-19 14:02       ` Josh Boyer
2007-02-19 14:04         ` Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 10/44 take 2] [UBI] debug unit implementation Artem Bityutskiy
2007-02-17 21:00   ` Arnd Bergmann
2007-02-19 12:29     ` Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 11/44 take 2] [UBI] allocation unit header Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 12/44 take 2] [UBI] allocation unit implementation Artem Bityutskiy
2007-02-17 20:55   ` Arnd Bergmann
2007-02-19 11:05     ` Artem Bityutskiy
2007-02-19 11:13   ` Pekka Enberg
2007-02-20 11:30     ` Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 13/44 take 2] [UBI] I/O unit header Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 14/44 take 2] [UBI] I/O unit implementation Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 15/44 take 2] [UBI] scanning unit header Artem Bityutskiy
2007-02-17 23:07   ` Theodore Tso
2007-02-18  2:17     ` Josh Boyer
2007-02-17 16:55 ` [PATCH 16/44 take 2] [UBI] scanning unit implementation Artem Bityutskiy
2007-02-19 11:05   ` Christoph Hellwig
2007-02-19 14:11     ` Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 17/44 take 2] [UBI] build unit header Artem Bityutskiy
2007-02-17 16:55 ` [PATCH 18/44 take 2] [UBI] build unit implementation Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 19/44 take 2] [UBI] volume table unit header Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 20/44 take 2] [UBI] volume table unit implementation Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 21/44 take 2] [UBI] background thread unit header Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 22/44 take 2] [UBI] background thread unit implementation Artem Bityutskiy
2007-02-19 11:09   ` Christoph Hellwig
2007-02-19 13:55     ` Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 23/44 take 2] [UBI] wear-leveling unit header Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 24/44 take 2] [UBI] wear-leveling unit implementation Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 25/44 take 2] [UBI] EBA unit header Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 26/44 take 2] [UBI] EBA unit implementation Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 27/44 take 2] [UBI] bad block handling unit header Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 28/44 take 2] [UBI] bad block handling unit implementation Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 29/44 take 2] [UBI] update unit header Artem Bityutskiy
2007-02-17 16:56 ` [PATCH 30/44 take 2] [UBI] update unit implementation Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 31/44 take 2] [UBI] accounting unit header Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 32/44 take 2] [UBI] accounting unit implementation Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 33/44 take 2] [UBI] volume management unit header Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 34/44 take 2] [UBI] volume management unit implementation Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 35/44 take 2] [UBI] user-interfaces unit header Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 36/44 take 2] [UBI] user-interfaces unit implementation Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 37/44 take 2] [UBI] sysfs handling unit header Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 38/44 take 2] [UBI] sysfs handling unit implementation Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 39/44 take 2] [UBI] character devices handling sub-unit header Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 40/44 take 2] [UBI] character devices handling sub-unit implementation Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 41/44 take 2] [UBI] gluebi unit header Artem Bityutskiy
2007-02-17 21:14   ` Arnd Bergmann
2007-02-18  2:04     ` Josh Boyer
2007-02-18  2:15       ` Arnd Bergmann
2007-02-18  3:02         ` Josh Boyer
2007-02-18 22:37           ` Arnd Bergmann
2007-02-19 13:52             ` Artem Bityutskiy
2007-02-19 14:01             ` Josh Boyer
2007-02-19 14:07           ` Jörn Engel
2007-02-19 12:29       ` Christoph Hellwig
2007-02-19 13:30     ` Artem Bityutskiy
2007-02-17 16:57 ` [PATCH 42/44 take 2] [UBI] gluebi unit implementation Artem Bityutskiy
2007-02-17 16:58 ` [PATCH 43/44 take 2] [UBI] JFFS2 UBI support Artem Bityutskiy
2007-02-17 16:58 ` [PATCH 44/44 take 2] [UBI] update MAINTAINERS Artem Bityutskiy
2007-02-17 22:49 ` [PATCH 00/44 take 2] [UBI] Unsorted Block Images Theodore Tso
2007-02-19 12:48   ` Artem Bityutskiy
2007-02-19 14:33     ` Theodore Tso
2007-02-19 17:07       ` Artem Bityutskiy [this message]
2007-02-19 23:34         ` Theodore Tso
2007-02-20 11:54           ` Artem Bityutskiy
2007-02-25  5:51         ` Christoph Hellwig
2007-02-26 10:11           ` Artem Bityutskiy
2007-02-19 10:50 ` Christoph Hellwig
2007-02-19 17:44   ` Artem Bityutskiy
2007-02-25  5:55     ` Christoph Hellwig
2007-02-20 14:52 ` John Stoffel
2007-02-20 17:41   ` Artem Bityutskiy
2007-02-20 17:44   ` Josh Boyer
2007-02-25  5:48   ` Christoph Hellwig

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=1171904866.14817.36.camel@sauron \
    --to=dedekind@infradead.org \
    --cc=dwmw2@infradead.org \
    --cc=haver@vnet.ibm.com \
    --cc=hch@infradead.org \
    --cc=jwboyer@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.