On Mon, Jan 08, 2024 at 10:36:59AM +0100, Dimitri Fedrau wrote:
> Rename mv88q2xxxx_get_sqi to mv88q2xxx_get_sqi and
> mv88q2xxxx_get_sqi_max to mv88q2xxx_get_sqi_max.
> Fix linebreaks and use everywhere hexadecimal numbers written with
> lowercase letters instead of mixing it up.
You could split is up into three patches. Its probably not worth it
now, but its something to remember for the future.
Ideally you want lots of small patches which are obviously correct. A
patch just containing a rename mv88q2xxxx_get_XXX to
mv88q2xxx_get_sqi_XXX etc, should be obviously correct, and just takes
a few seconds to review.
A patch adding a few line breaks should again take a few seconds to
review.
Upper case to lower case is easy to review.
When it is all mixed together, in a bigger patch it takes a bit more
effort to review, a bit more effort is needed to look for typ0s etc.
Its can be faster and easier to review 10 very simple patches than 3
big patches...
Andrew
Am Mon, Jan 08, 2024 at 03:02:18PM +0100 schrieb Andrew Lunn:
> On Mon, Jan 08, 2024 at 10:36:59AM +0100, Dimitri Fedrau wrote:
> > Rename mv88q2xxxx_get_sqi to mv88q2xxx_get_sqi and
> > mv88q2xxxx_get_sqi_max to mv88q2xxx_get_sqi_max.
> > Fix linebreaks and use everywhere hexadecimal numbers written with
> > lowercase letters instead of mixing it up.
>
> You could split is up into three patches. Its probably not worth it
> now, but its something to remember for the future.
>
> Ideally you want lots of small patches which are obviously correct. A
> patch just containing a rename mv88q2xxxx_get_XXX to
> mv88q2xxx_get_sqi_XXX etc, should be obviously correct, and just takes
> a few seconds to review.
>
> A patch adding a few line breaks should again take a few seconds to
> review.
>
> Upper case to lower case is easy to review.
>
> When it is all mixed together, in a bigger patch it takes a bit more
> effort to review, a bit more effort is needed to look for typ0s etc.
> Its can be faster and easier to review 10 very simple patches than 3
> big patches...
>
It makes totally sense to me. Haven't thought about it. Thanks for
your explanation. I will keep it in mind for future patches.
> Andrew
Best regards,
Dimitri