[Scummvm-devel] Engine merges

[Willem Jan Palenstijn]

Hi all,

With a number of engine merges over the past few years I'm increasingly getting
the feeling that sometimes merge reviews are somehow considered an impediment
or a formality, rather than a way to ensure and improve code quality.

With the ever-increasing number of engines, and the currently fairly constant 
number of developers, the merge review is for many engines likely the only time
it gets properly looked at in any depth by other people than the main engine
developers.

Since reviewing large bodies of code takes quite a lot of time and energy, it
is really frustrating to me that some merges seem needlessly rushed, instead of
us taking the time to properly go over them, especially since we're not on any
tight schedule, or forced by any external deadlines. And, with git being
distributed, work can continue on engines even during a review period.

(Many merges are different, though, and I really appreciate everyone taking the
time to take our -- sometimes very extensive -- comments into account!)



We've discussed these things in the past, and had created the following
section on the wiki for this:

http://wiki.scummvm.org/index.php/HOWTO-Engine_Inclusion#The_merge_discussion


While I don't want to dwell too long on explicit rules, I would like to mention
that for today's Access engine merge, at least three points were not followed:

* The majority ScummVM core team members have to *explicitly* state that they
  are fine with the merge

* All other core team members have at least told their stance on the merge.

* How we proceed from then on will be decided on a per case basis.

On top of that, just hitting the Merge button effectively in the middle of a
discussion is bad form.



-Willem Jan


P.S. I don't mean to imply anything about the quality of the Access engine,
because I simply haven't had the time to give it more than a quick glance with
the end of year deadlines at work.