[Mono-dev] RE: [Mono-patches] r57149 - in trunk/mcs/class/System.Data: System.DataSystem.Data.Common Test/ProviderTests/System.Data.SqlClientTest/System.Data

[Miguel de Icaza]

Hey,

> It sounds reasonable only if everyone is ready to "sign"
> certificates for each of non-trivial patches so that he or she
> has assured that each does not break anything.
> 
> I don't think such "review" work well. It should be fine if someone
> who broke tests fix the problem, or just revert in case it was
> impossible. That's what has happened to such patches that are
> accompanied by "please review the patch" posts.
> 
> The same amount of "massive" patches happen to mcs/gmcs land
> without any approvals from the maintainers, kinda everyday.

Well, the fact that it happens does not mean that we condone the
practice.

Usually for smaller things, its ok to use the "asynchronous" review
process: things are committed, and we get to review them after the
commit (we still assume that things do not break the build and that
people have tested the changes before committing).

Every once in a while regressions happen, and we should be ready to cope
with those regressions rapidly.  

My point is: the fact that it happens does not mean its a good idea, or
that we embrace the breakage.