kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 05/11/2012 01:57 AM, Lorenzo Marcantonio wrote:
> Keeping alterations separate is a lot of work, since refactoring tends
> to be a recursive process


Agreed, it is a lot of work now.   (So is reading and approving a big patch.)


One way to do this refactoring work, future-istically speaking, is to create a branch from
a checkout, and then commit to the that branch at intermediate points.


Then go back and make patches from the revision deltas, you can pick any two revisions and
make a patch from those two revisions, they do not need to be consecutive:


$ bzr diff -r 3445..3445 > first.patch




> ... anyway, these are the first 'palatable'
> patches:




>
> 001     why the PROPERTIES hash has char* instead of const char* as
>         index? I get deprecation warning on usages like m_prop["header"]

We had this conversation already.  Again, const char* does not compile here.

I said I would look at it when I got to 4.6 gcc, again.  If the wx headers cannot provide
a portable hashtable, we'll look elsewhere.


>
>         similarly I get ambiguous warning on the + '\n' + usage, which
>         is easily fixed with wxChar( '\n' ) (the message is funny, too)
That's a good fix.


>
> 002     MIN_PAGE_SIZE and MAX_PAGE_SIZE from #define to const inside
>         PAGE_INFO. 
>
>         s_gost became is a const
>
>         The ON and OFF defines replaced with the obvious true and false
>         bool
>
> 003     Timestamps (the numeric ones) became time_t (before they were
>         a mix on long/unsigned long/int... why some other timestamps are
>         wxDateTime? the netlist reader uses directly the hex string
>         form, too
>
> 004     The various flags in EDA_ITEM (m_status and m_Flags... why two
>         sets?) became and enum in EDA_ITEM
>
> 005     Two relic defines from bitmaps_h removed
>
> 006     Two const added
>
> 007     Renamed BASE_SCREEN::m_NumberOfScreen to
>         BASE_SCREEN::m_NumberOfScreens (otherwise you get mad
>         with m_ScreenNumber too!)
>
>         Removed BASE_SCREEN::m_FlagRefreshReq which was wholly unused
>
>         Some const
>
> 008     MODULE::m_LastEdit_Time became a time_t too. Also added an
>         overloaded SetLastEditTime member which sets it to the current
>         time (the most frequent usage)
>
> 009     The drawmode (GR_COPY, GR_XOR and so on) became an enum. Luckily
>         wxCOPY has probably the same zero value of GR_COPY since it was
>         used a couple of time
>
> 010     EDA_COLOR_T made pervasive (an enum can always be assigned to
>         an int but the reverse in newer g++ is an *error*)

Wayne seems to be taking the lead on this, so I will now go silent for the time being.

Dick