On Wed, Aug 18, 2010 at 11:54, Sascha Silbe <sascha-ml-ui-sugar-de...@silbe.org> wrote: > Excerpts from Tomeu Vizoso's message of Wed Aug 11 12:22:00 +0200 2010: > >> any code contributor is expecting that their patches will be tested by >> the reviewer? > A contributor should test their code to the best of their ability. > A reviewer can, but doesn't have to test the code. A tester can, but > doesn't have to review the code. With the mailing list based review model, > this is expressed as the separate Reviewed-By and Tested-By tags. > >> Because of this specific commit, file transfers have been broken since >> early this year and it's obvious that this code wasn't tested at all: >> >> http://git.sugarlabs.org/projects/sugar/repos/mainline/commits/11828796 > > Me being the original author of the patch, it's obvious that the code was > tested rather well before submitting it for review. But > a) I'm not perfect, so some code paths were not exercised before the > initial submission for review > b) it took many months before the patch was accepted, so it bitrotted. > Simon probably had to solve a number of conflicts; also some changes > in the surroundings of the patch might have been "ignored" by the VCS. > All this is an error-prone process that requires any test to be > re-run / re-done. > > > This is a very nice example of > a) how the long time patches wait in the review queue and the cumbersome > process of submitting them to Trac is negatively affecting the project > (I remember fixing an issue in this very patch related to file sharing, > but probably never submitted an updated patch to Trac because it was > rejected anyway back then). > > b) why automated testing is crucial. (I don't think I need to elaborate > that point, it should be obvious from the above if it wasn't obvious > before already.) > >> Given the current poor state of our testing efforts, > The best way to get Sugar tested well (besides automatic tests which only > catch specific cases) is by eating our own dog food. Of course, this > requires us to accept some "high ceiling" patches (that no deployment > will call for) instead of just "low floor" ones. This might increase the > risk of breaking something (or raising the floor), but IMO is more than > offset by the better testing and increased number of motivated > contributors we get.
You make good points, but the bit about dogfooding doesn't hold as well because I, for example, don't ever use file transfer in GNOME. In any case, I don't think you are saying that if I propose a patch that changes code in the file transfer path I shouldn't have tested file transfers? Btw, I've been looking at AT-SPI with eyes on accessibility and testing today. Regards, Tomeu > > Please consider this mail food for thought, not a direct retort to your > complaint about the breakage. That I was the one who authored this > original patch is merely a detail showing that even careful and > experienced contributors (as I consider myself to be) can't be relied upon > to not make mistakes. We need to accept the fact that humans are imperfect > and cater for it by tuning our processes and goals. > > Sascha > > -- > http://sascha.silbe.org/ > http://www.infra-silbe.de/ > > _______________________________________________ > Sugar-devel mailing list > Sugar-devel@lists.sugarlabs.org > http://lists.sugarlabs.org/listinfo/sugar-devel > > _______________________________________________ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel