[PATCH] Substitution of VIA_VM800 label with VIA_P4M800PRO label

Benno Schulenberg bensberg at justemail.net
Thu Jan 7 13:05:50 PST 2016


It is customary to have the subject line of a patch in the imperative,
so here: "substitute the label VIA_VM800 with VIA_P4M800PRO".

On Thu, Jan 7, 2016, at 01:14, Kevin Brace wrote:
> Replaces VIA_VM800 label with VIA_P4M800PRO label

Redundant.  It repeats the subject line.  Unneeded, annoying.

> inside via_driver.c

Totally superfluous.  This is what git is for, what git can produce
these nice ++++---- tables for.

> due to the fact that

Unneeded words.

> there is no such product as VM800 chipset from
> VIA Technologies.

Right.  That is the essential information, and fully suffices.

> Of course, references to VIA_VM800 label in other
> files were also replaced as well.

More than superfluous.

> The VM800 chipset the source code
> refers to really is P4M800 Pro chipset.

Yes, otherwise you wouldn't make the replacement.
So, again, superfluous.

> Also, the source code refers
> to a product called VN800 chipset, and this is similar (related) to
> P4M800 Pro chipset.

Unrelated info that doesn't make things clearer.

>     The compiled device driver was tested on the following computer.
> 
> - Epic 1314 laptop (MSI VR321 laptop equivalent, VN896 chipset)
>   with Lubuntu 12.04 i386

Completely unneeded info.  You only changed a label, you made
no functional changes, so of course things are still working.
Plus: that laptop doesn't even contain a P4M800Pro.


> -        /* P4M800Pro/VN800/CN700 */ 
> -        case VIA_VM800:
> +        /* P4M800 Pro/VN800/CN700 */ 
> +        case VIA_P4M800PRO:

Why add the space in the comment?  It makes it seem as if Pro and
VN800 are alternatives.  And it would be nice if, when grepping,
one needs to grep -i for just one form of M800Pro to find all
occurrences.

ec VIAChipsets[] = {
>      {VIA_KM400,    "KM400/KN400"},
>      {VIA_K8M800,   "K8M800/K8N800"},
>      {VIA_PM800,    "PM800/PM880/CN400"},
> -    {VIA_VM800,    "VM800/P4M800Pro/VN800/CN700"},
> +    {VIA_P4M800PRO,    "P4M800 Pro/VN800/CN700"},
>      {VIA_CX700,    "CX700/VX700"},
>      {VIA_K8M890,   "K8M890/K8N890"},
>      {VIA_P4M890,   "P4M890"},

Why not keep the vertical alignment?

Benno

-- 
http://www.fastmail.com - Choose from over 50 domains or use your own



More information about the xorg-devel mailing list