kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #08241
Re: [PATCH] Reworking patchlets, first issue
On 5/11/2012 9:06 AM, Dick Hollenbeck wrote:
> 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.
I'll take care of committing these patches one at a time. Once you get
them to the point where they can be applied to the latest revision of
the testing branch, please post them to the mailing list for further
comment. Your first patch was difficult to get a handle on because it
such a large change set. I'm almost sure I missed some things due to
it's size. Once everyone is OK with the changes and I don't find any
problems in my testing, I will commit them. I am extremely busy right
now so please be patient as I try to process them.
Wayne
>
> Dick
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help : https://help.launchpad.net/ListHelp
>
Follow ups
References