Re: Fwd: [PATCH 1/2] Framebuffer split

[Vladimir 'phcoder' Serbinenko]

On Tue, Jul 28, 2009 at 7:50 PM, Robert Millan<address@hidden> wrote:
> On Mon, Jul 27, 2009 at 12:06:17AM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> Removed parts which are purely moving code around (of course it will
>> be restored on commit).
>
> This is not a full review of the patch, just a quick overview.  But I have
> a few comments:
>
>  - The "fb" naming is confusing.  "framebuffer" refers to a Linux API
>    exported to userland on GNU/Linux, but it seems to be used for
>    referring to the backend.  I think it's better if we call it just
>    that ("backend").  Or maybe "driver" but that doesn't apply well to
>    vbe.
backend and driver aren't good names because it's neither a complete
backend nor a driver. It's more like a functions library which can be
used by drivers. Prefix video_fb was chosen deliberately to avoid
names to become too long. I don't think that video_fb is of any more
problem than video_vbe since FB is a widely used abbreviation when
speaking about graphics.
>
>  - Some copyright years need updating (I noticed video_fb.h, but please
>    review the other files as well).
>
Thanks
>  - It'd help if the patch could actually be split in standalone parts
>    that can be committed separately.  Maybe this is not possible, but
>    at least for some parts of it, it probably is.
>
Some parts yes but the main part of patch is dispatching functions
from vbe.c and splitting it may cause intermediary states to be easily
broken.
> --
> Robert Millan
>
>  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>  how) you may access your data; but nobody's threatening your freedom: we
>  still allow you to remove your data and not access it at all."
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git