public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Igor Grinberg <grinberg@compulab.co.il>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 3/4] common/lcd_console: move single static variables into common (static) structure
Date: Wed, 18 Mar 2015 14:47:09 +0200	[thread overview]
Message-ID: <550973CD.8010501@compulab.co.il> (raw)
In-Reply-To: <55096C16.3010907@petermaier.org>

On 03/18/15 14:14, Hannes Petermaier wrote:
> On 2015-03-18 13:11, Igor Grinberg wrote:
>> [...]
>>> +struct console_t {
>>> +    short curr_col, curr_row;
>>> +    short cols, rows;
>>> +    void *lcd_address;
>> Shouldn't this be fbbase?
>>
>>> +};
>>> +static struct console_t cons;
>> [...]
>>
> Hi Igor,
> my plan was/is to make a cleanup patch after this story is closed.
> - remove empty lines in code

Yes indeed, but...

> - rename some variables

This depends. For example the lcd_address, you are introducing
a lot of new places where this variable is used.
So, IMO, it is better to rename it before you are using it, otherwise
the renaming patch will be much bigger and that is called needless churn...

> 
> would it be better to do that immediately ?

Well, I think that there are certain levels of cleanup...
In case of the lcd_address, I think it is better to rename it before use.

I think the rule of thumb (meaning this is not cast in stone) is first
do a clean up and then add a functionality to already clean code.

-- 
Regards,
Igor.

  reply	other threads:[~2015-03-18 12:47 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-18  7:37 [U-Boot] [PATCH v2 0/4] common/lcd_console: introduce display/framebuffer rotation Hannes Petermaier
2015-03-18  7:37 ` [U-Boot] [PATCH v2 1/4] common/lcd_console: cleanup lcd_drawchars/lcd_putc_xy Hannes Petermaier
2015-03-19  8:37   ` [U-Boot] [PATCH v3 0/4] common/lcd_console: introduce display/framebuffer rotation Hannes Petermaier
2015-03-19  8:37     ` [U-Boot] [PATCH v3 1/4] common/lcd_console: cleanup lcd_drawchars/lcd_putc_xy Hannes Petermaier
2015-03-19  8:37     ` [U-Boot] [PATCH v3 2/4] common/lcd_console: ask only one-time for bg/fg-color per call Hannes Petermaier
2015-03-19  8:37     ` [U-Boot] [PATCH v3 3/4] common/lcd_console: move single static variables into common (static) structure Hannes Petermaier
2015-03-19  8:37     ` [U-Boot] [PATCH v3 4/4] common/lcd_console: introduce display/framebuffer rotation Hannes Petermaier
2015-03-25 16:24       ` Nikita Kiryanov
2015-03-25 21:24         ` Hannes Petermaier
2015-03-26 16:32           ` Nikita Kiryanov
2015-03-18  7:37 ` [U-Boot] [PATCH v2 2/4] common/lcd_console: ask only one-time for bg/fg-color per call Hannes Petermaier
2015-03-18  7:37 ` [U-Boot] [PATCH v2 3/4] common/lcd_console: move single static variables into common (static) structure Hannes Petermaier
2015-03-18 12:11   ` Igor Grinberg
2015-03-18 12:14     ` Hannes Petermaier
2015-03-18 12:47       ` Igor Grinberg [this message]
2015-03-18 15:07         ` Hannes Petermaier
2015-03-18 15:23           ` Igor Grinberg
2015-03-18  7:37 ` [U-Boot] [PATCH v2 4/4] common/lcd_console: introduce display/framebuffer rotation Hannes Petermaier
2015-03-18 12:56   ` Igor Grinberg
2015-03-18 14:59     ` Hannes Petermaier
2015-03-18 15:27       ` Igor Grinberg

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=550973CD.8010501@compulab.co.il \
    --to=grinberg@compulab.co.il \
    --cc=u-boot@lists.denx.de \
    /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