[Scummvm-devel] [Scummvm-cvs-logs] SF.net SVN: scummvm:[55130] scummvm/trunk/configure

[Johannes Schickel]

On 01/07/2011 01:59 PM, Max Horn wrote:
> * What else do you consider a pro for merging engines to master early on? Extra helping hands? Well, you can get those on your git fork of master, too, it's very easy for them to work on your tree, isn't it? Extra code review? Well, I scaled my code review back on new engine code, because I got yelled at too often for it: "Stop complaining, this is code in progress and we'll rewrite it later on anyway!". OK OK, but how am I supposed to know when and when not code review is permissible? Heck, in fact, I think all of us have a right to do code review on *anything* that goes into master at any time, without ever having to justify that.

Seriously on this one: How often did the people rewrite the code later 
on? I personally doubt too many did in the end... (that's based on 
personal observations of myself when I say that though).

I agree if codes goes into master there is no reason why it should not 
get reviewed. It is actually worrying that people complain when someone 
is doing a code review, usually code reviews rather help in my 
experience. Actually I think the best argument for having code in a 
public development tree is code review anyway....

> Long rant, short end: I am OK with being very permissive about adding new engines, *if* we setup some rules for that. Here is a suggestion:
>
> Every new engines would begin its life in "disabled state" on master.  Authors of new engines agree to ensure their engine gets completed. We define what that means (game completable with at most minor glitches, sufficient integration into the ScummVM framework, endian safe, works on multiple platforms; which platforms of course depends on the specs of the games).
>
> To achieve this, one or more persons become official "maintainers" of the engine. This may be distinct from the list of engine authors.
>
> An engine in "disabled" state always must have maintainers. If you can't work as maintainer anymore, it's *your* job to find a successor (of course as others may help, if they feel like it, but it's the maintainers duty). If you don't find a successor (because there is none, or because you don't have time, or because you just don't care, which of course you are free to, as a volunteer), we flag the engine as "unmaintained" for a certain period, say 6 months. The team may decide during this time to try to find new maintainers from inside or outside, but doesn't have to. If no maintainer is found during that time, we remove it from master again. If a new maintainer is found, we give a probation time, during which some progress should happen, else we will remove the engine anyway.
>
> Once the engine is deemed "complete", it enters the "enabled state" phase of its life. The role of maintainers now changes: They now promise to maintain the working engine, feeling responsible for addressing bug reports. Doesn't mean they have to do all the work; just that you then would feel responsible for either fixing the bug yourself, or finding somebody to do it, etc.
> If this maintainer becomes unavailable without successor, we then once more flag the engine as unmaintained, but since it is completable, we will keep it, and keep it enabled. We would maintain a list of "unmaintained" engines somewhere, with a permanent "job offer" written there, and maybe e.g. in the SF.net "jobs" system and other places. If we undertake serious refactoring that require some knowledge of the engine to test it, however, we might have to demote it from "enabled" to "disabled" state. I'd like to avoid this as much as possible, but if we just can't find anybody to fix it, and adapt it to code changes, and to test it, well, we gotta drop support. If that happens, I would suggest keeping it in the repository anyway, since it used to be enabled; and to try to find a new maintainer.
>
> A similar approach could be done for ports.
>

Personally I think this is more or less similar to what we do for 
existing ports already, except that we do not explicitly have the 
maintainer rules.

Actually I am all for applying similar rules (as you suggested) on 
engines. At least I see no reason why after changes porters need to 
worry about getting their ports to work again themselves mostly, while 
for engines it is more or less an unspoken rule that if someone does 
changes on common code they need to adapt *all* the engines to it too.

I am not saying that when you do changes on common code you should not 
have any obligations on getting client code adapted, but I think 
everyone who worked on the common code and had to adapt more than 5 
engines will agree with me, that it is sometimes getting tiresome to dig 
into too many engines. Especially when nobody with knowledge of the 
engine is around...

I would actually suggest that we introduce some minimal requirements on 
what code one has to adapt when changing common code. It seems 
reasonably to me that we limit it to maintained official engines, since 
on these you usually have a person with knowledge about the code, i.e. 
the maintainer, which makes adapting the code much easier. Since 
unofficial engines are in development anyway, I think it is fine to let 
the maintainer/developers worrying themselves about it. Of course one 
should at least answer their questions in this case, so they are not 
totally lost here.

// Johannes