On Tue, Mar 23, 2010 at 10:48 AM, Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> wrote: > Devin Heitmueller wrote: >> On Tue, Mar 23, 2010 at 9:45 AM, Mauro Carvalho Chehab > >> And as I explained to you, there were *extraordinarily* good reasons - >> because the code will be enabled in the future, the code definitely >> didn't belong in "ngene-core.c", and because I didn't want the code to >> get lost entirely. > > It won't get lost. It is forever stored on -hg history. > >> Splitting the code out into different files has >> *huge* benefits in parallel development - for example on Saturday I >> spent the whole day focusing on the V4L2 aspects of the ngene bridge >> while Steven spent the day fixing bugs in the code related to IRQ >> handling and the i2c stack. At the end of the day, I did an hg PULL >> with zero conflicts. >> >> And if we are showing people the patches, let's show them the *actual* >> patch as submitted before your scripts tore it apart: >> >> http://kernellabs.com/hg/~dheitmueller/ngene2/rev/baa9613caeb7 >> >> See? Nice, clean concise. If anyone had looked at that patch they >> would have said, "Ok, that makes sense". When somebody is ready to >> make that code work, they can just uncomment it and fix the bugs. > > It keeps make no sense to add a /dev/null file into the building system. > This just slows down kernel compilation. > >> Who in their right mind tells a developer to, "waste four hours >> redoing a patch series because one of the patches gets mangled by my >> scripts to the point where it is a no-op in the upstream kernel?" >> >> This could have been addressed in fifteen seconds if you had just >> added a comment to the patch that said, "comes out as a no-op due to a >> merge issue with keeping the hg in sync", and I seriously doubt anyone >> would have given it a second thought. Instead, we are making crappy >> engineering decisions to appease the gods of "patch cleanliness >> extremism". > > This has nothing to do with hg sync. With or without the #if 0 block, it > is still a do-nothing patch. As you said me yesterday on irc that you > currently have no plans to work on the comment code, this means that > the dead code would stay on kernel upstream for a long time. > >> Penalty in the build system? Are you kidding? *THAT* is your >> argument? The extra 10ms it takes to compile a no-op file versus the >> hours it would take to to rework the patch and the loss of the code >> from the tree? > > Kernel has about 6000 developers. Imagine that each one wants to add their > own 322 lines do-nothing files there, adding those do-nothing files at the > building system, with lots of includes that will never be used, just because > this code might be useful by someone, some day. > > I don't think this would scale. No need to take it to extremes. Nobody has claimed that empty files are always a good idea. There are cases where it makes sense though. If the git tree is *really* to become the place where development happens (with hg being for backports only), then you're going to have to accept that the tree is going to contain some test code. The reality is that between Steven and I, we are going to be putting several hundred hours of work into this driver to take something that "barely works today" and turn it into something that is a commercial quality solution. And there are going to be a *ton* more patches. With only two or three days worth of work, we've already authored 29 patches, and we're just getting started. There are likely going to be *hundreds* of patches being submitted for this driver. That said, if getting even trivial changes like moving a few functions around are going to be met with such resistance and come at an enormous cost, it's *very* tempting to just host it locally and not submit it upstream at all. In the end, you forced me to do three or four hours of work with *ZERO* tangible benefit. The code is no better as a result, no bugs were resolved. All so that you could avoid one patch whose purpose wasn't clear in the git version. Don't let the community suffer due to your unwillingness to compromise. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
