Re: [flashrom] [PATCH 1/3] layout: change return type and name of find_next_included_romentry

[Tadas Slotkus]

On Pr, 2011-08-15 at 06:43 +0200, Stefan Tauner wrote:
> rename from find_next_included_romentry to get_next_included_romentry
> return a pointer to a rom_entry instead of just its index
>
> Signed-off-by: Stefan Tauner

Acked-by: Tadas Slotkus

[PATCH 1/3] layout: change return type and name of find_next_included_romentry

[Stefan Tauner]

On Mon, 15 Aug 2011 17:34:59 +0300
Tadas Slotkus  wrote:
> On Pr, 2011-08-15 at 06:43 +0200, Stefan Tauner wrote:
> > rename from find_next_included_romentry to get_next_included_romentry
> > return a pointer to a rom_entry instead of just its index
> >
> > Signed-off-by: Stefan Tauner
>
> Acked-by: Tadas Slotkus

hello tadas

nice to see someone reading my patches!
but TBH the three reviews look a bit superficial as far as one can
judge that from the mails alone (which might be wrong and in that case
please accept my apologies).

you can't just ack a patch named "an obvious but *dubios* refactor
opportunity" without any discussion for example :)
why do you think it is worth it and should be included?
also, see my mail regarding 1/3.

i am not much involved in coreboot development, but in flashrom we are
very picky about almost any change. :)
we think this helps code quality a lot. OTOH our patch queue is so long
because we don't have enough time to review. rigorous reviews take a
lot of time and effort. if you are not entirely sure that a change
should be committed, just tell what you think without appending your
acked-by.

that all said, i appreciate any comment regarding my patches very much.
thank you!

[PATCH 1/3] layout: change return type and name of find_next_included_romentry

[Tadas Slotkus]

> hello tadas
>
> nice to see someone reading my patches!
> but TBH the three reviews look a bit superficial as far as one can
> judge that from the mails alone (which might be wrong and in that case
> please accept my apologies).
>
> you can't just ack a patch named "an obvious but *dubios* refactor
> opportunity" without any discussion for example :)
> why do you think it is worth it and should be included?
> also, see my mail regarding 1/3.
>
> i am not much involved in coreboot development, but in flashrom we are
> very picky about almost any change. :)
> we think this helps code quality a lot. OTOH our patch queue is so long
> because we don't have enough time to review. rigorous reviews take a
> lot of time and effort. if you are not entirely sure that a change
> should be committed, just tell what you think without appending your
> acked-by.
>
> that all said, i appreciate any comment regarding my patches very much.
> thank you!

Hi,

yep you are right, anyways I like the main idea. At least it makes more
clear what is going on.

Thanks,
Tadas