← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH 01/19] Replace DrawPinShape enum with PinShape

 

Le 22/02/2016 14:40, Simon Richter a écrit :
> Hi,
> 
> On 19.02.2016 18:27, jp charras wrote:
> 
>> Could you modify your changes to be compliant with our coding
>> style policy, when changing enum names?
> 
> Sure -- do we really want enum names to be uppercase like class
> names, or is that going to lead to confusion?

We try to have only one coding style.
Currently the coding style for enum is (see coding-style-policy.md):

"## 2.1 Class, Type Definitions, Name Space, and Macro Names ##
{#definitions}
Class, typedef, enum, name space, and macro names should be comprised of
all capital letters."

Confusion exists more surely when there is more than one coding style
in sources.

Of course, due to the fact a lot of code was written before the coding
style doc, you can find enums and class names in lower chars.
But when modifying the enum or class names, or creating new
enum/classes, using our Kicad current coding style is a good thing.

Confusion can be minimized by good names (too short names create
confusion), for instance some enums used to define a type of item use
names line PIN_TYPE_T  (_T for item Type, in this example).

I do not yet know a coding style which is able to avoid any confusion.

> 
>> Also can you group yours patches relative to the same thing (5
>> patches to move an enum in a header and change its name is too
>> much for me).
> 
> Can do -- my goal was to keep the reviews short and independent -- 
> applying them in order gives a working state after every patch, so
> if there is only time to look at one, this doesn't break anything,
> and as long as I keep them separately in my repository, I can also
> still easily rebase stuff in case one of them is rejected.
> 
> The general direction I'm heading off is:
> 
> - keep enum values in a header of at least similar name so they can
> be easily found - keep UI code (lookup of localized names and
> bitmaps) separate from data structures - create dedicated widget
> classes that initialize themselves and enforce typesafe accesses
> 
> If all of these sound good, I can merge the changes to have fewer 
> commits in total.
> 
> Simon

Currently I have no objection.

-- 
Jean-Pierre CHARRAS


References