Daniel,
First off, it's really good to see you committing. It's much
easier to help you when everyone can see what you're doing. I think
you're basically on the right track with the concept of your changes.
But... now for the bad news.... :(
Regarding the actual commits in this branch, it's really not
going well. At all. There are really too many things wrong with your
recent commits to go into, so I think it's better to look at the big
picture. You should know that it's quite common for us to start one
branch just to figure our how to do something right, and then "redo"
the branch in order to actually do it right.
If that's what you're doing, that's fine. However, you should
know that the commits on the current branch will never be merge-able.
In fact, the commits on the branch so far suggest that you might not
realize what the standards are for a branch to be merge-able. It might
be pretty disappointing for you to only figure that out later, and
since they're not explicit in the docs, I want to try to be more clear
about them. And now, since you've committed some code, we have some
examples for reference.
The _goal_ is to have a branch of small, logical,
well-documented changes, such that each revision compiles, and works.
(In exceptional cases, there might be minor, well-understood
regressions.)
Of course, when you start, you don't know what that set of
changes will look like, but here's the strategy I recommend (and use)
to figure it out:
1) Make a single logical change that's fundamental to your final
goal. In this case, it might be making QofEntity a Gobject, or
combining QofEntity into QofInstance, or, possibly, combining
QofEntity into Qofinstance and making it a GObject. Although, that
might be too big to qualify.
2) Identify the _minimum_ set of changes to make the first change
compile and work.
3) Classify all the changes in 2) into two groups: A) those that
don't depend on the change in 1); and B) those that do. An example in
group A) would be changing a use of "acc->inst.entity" to
"QOF_ENTITY(acc)." An example in group B) would be using some new
GObject-provided feature of the class. Another example would be using
qof_instance_get_kvp_data(acc) instead of acc->inst.kvp_data.
4) If all the changes in A) are small, preparatory changes, then
prepare commits for them at the beginning of the sequence. If any of
the changes in A) is itself a more fundamental change, possibly with
its own prerequisites, then put everything on hold, and restart the
process with that change as step 1). Often, group A can be broadly
characterized as "using existing abstractions that should've be used
all along".
5) Now, look at group B. This group of changes is problematic,
because their dependence on #1) implies that you can't make change #1)
without making them, too. But, their absence from #1 implies that
they are not logically fundamental to the change in #1. This group
must be eliminated, by either
5.1) deciding that the change really is fundamental to #1, and so
should be included in #1, or
5.2) realizing that the change doesn't really depend on #1, but
instead is a desirable, but, strictly speaking, optional, consequence
of #1, or, more usually,
5.3) changing #1 in some way to eliminate the need for the change.
In the end, group B usually turns out to be broadly characterized
as "introducing new abstractions that decouple the code I want to
change from the code that depends on the code I want to change." And,
most importantly, it can now be ordered before or with #1.
In this specific case, step 5) will probably involve introducing
lots of "ugly" #defines and typedefs. For example,
"#define QOF_ENTITY QOF_INSTANCE". That's okay.
Let me just throw out a wild, off-the-cuff estimate what your first
merge-able branch should look like. 95% of the changes would be under
libqof. It would introduce 100 new #defines. It would have 30-50
revisions, and half of them could theoretically be made in trunk with
no breakage. It would result in the top-level objects being GObjects,
but not the core engine objects yet, except insofar as the engine
objects are struct-overlays of GObjects. It would work exactly as
well as trunk.
Perhaps you don't realize that that's all that would have to be
accomplished before we'd want to merge it into trunk. We know you'd
want to take it much further, but you can have as many branches as you
want. The more, the better. You probably have plans enough to make
4-5 more branches just as substantial as the first.
Also, let me say, yes, I know this is tedious, painful work,
especially when you're confident that you can just "make it work".
But, it's the price we all pay in order to work together even though
we don't share the same brain. And, it's important.
I don't see how this line of development is going to help, even
when trying to create mer-gable branch. The commits are just way too
big, and 80% of these changes shouldn't even be in the first merge to
trunk.
Daniel, my advice is to start over. If it helps, pretend that you
are forbidden from changing identifiers, at least for the first round.
Don't be afraid to #define your way out of an API change. I know it's
tempting to "fix" all the bad identifier names. Don't do it.
Consider that every change outside of libqof should probably be
completely justifiable on its own merit.
I hope you take this criticism as a challenge to redo these changes
"right". We really do want the GObject integration that you're
achieving here, so I really hope you can start over and show us
something that is even close to merge-able.
-chris
p.s. When I've had to do major changes like this in the past, I've
found 'quilt' to be useful. http://savannah.nongnu.org/projects/quilt
_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
