public inbox for linux-kernel@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox