lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Code review: product editor


From: Evgeniy Tarassov
Subject: Re: [lmi] Code review: product editor
Date: Fri, 2 Mar 2007 16:30:27 +0100

On 3/2/07, Greg Chicares <address@hidden> wrote:
On 2007-3-2 10:59 UTC, Evgeniy Tarassov wrote:
> On 3/1/07, Greg Chicares <address@hidden> wrote:
>> (1) Class template MultiDimTableTypeTraits duplicates lmi's
>> function template value_cast(). I would much rather use the lmi
> [...]
>> (2) I'd prefer to use standard types instead of wx types where
>> possible. IIRC, wx container types are just synonyms for STL
>> containers anyway, but the string types are not substitutable.
>
> Sorry for that. MultiDimTableTypeTraits removed and value_cast used
> everywhere, where a conversion is needed.

Where should these changes be made? Note that I already merged
'product-editor_branch' into HEAD. As of 20070221T0307Z, IIRC,
the branch exactly matched HEAD, but I've made many changes in
HEAD since then.

Oh, I should have missed the merge event. All that time I was blindly
committing into product-editor_branch, which should be considered as
dead now, right?

(A) One way is to send a patch to me by personal email, as a
'.tar.bz2' file (inline patches can get mangled, especially
if you're using gmail). This may make the most sense for these
particular changes right now.

I will prepare a patch with all the changes made and send it to you asap.

Or maybe I should create another branch of HEAD, perform there all the
changes from product-editor_branch (since 20070221T0307Z - the time of
merge into HEAD)?

(B) Another way would be to think of that branch as "yours"
and HEAD as "mine", and work together to resolve differences.
I think this is probably the best way, because we're going
to be working together on this for a while. The big advantage
is that each of us has complete liberty to commit anything we
want to, without delay (in different places). The challenge
it poses is that we both have to work to review each other's
changes promptly.

Oh, IMHO it is getting really complicated. Do you think that we could
create a separate branch for product editor (again) so that both of us
could make all the changes related to product editor in there? And
once the code is stabilized to merge it back into HEAD -- exactly as
we've done it before. The advantage is that the process of reviewing
each other changes is the standard cvs process of updating the code.
If we separate the work into two parts in two different places then we
will be forced to synchronize (personal) branch after every little
change in another branch. I never did it this way before, so maybe I'm
just little conservative and it is not so complicated as it seems to
me.

[...]
If you can think of another way that would produce better
results, then I'm glad to listen. To me, "better" means a more
robust system, because what we really want is a rock-solid

If we carefully review each-other changes then no matter how we do it
should produce the same results. Maybe I'm missing something big, but
i dont really understand the advantage of making changes to the same
code (product_editor) in two different places. We will only benefit
from working in the same branch. Or maybe I'm really missing
something.

Anyway lets do it the way _you_ think the best for lmi. Just let me
know how do you want to do it -- I'll stick to it.

> wxString is replaced by
> std::string where possible. There are however two occurences of
[...]
Based on the experiments I've done so far, I think the rule
should be to use std::string unless that makes it impossible
to compile at all. Taking that as a postulate, I suspect that

Ok, thank you! I will fix the rest of the appearances of wxString.

Bear in mind that we might have
  class B            {virtual foo(wxString const&);};
  class D : public B {virtual foo(wxString const&);};
so, if we change (only) class D first, it won't compile, but
we really want to change both B and D.

Yes, I'm usually grepping the code for all the appearances of the
function name to make sure base class version is also updated.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]