On Tue, Mar 23, 2010 at 9:45 AM, Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> wrote: > Devin Heitmueller wrote: >> Ok, here's take two of the PULL request issued yesterday. It's >> basically the same as yesterday, but except instead of moving the >> unused code to separate files where it might actually be useful to >> someone else in the future, I delete it entirely because Mauro's >> scripts mangle the patches when pulling them into git (therefore the >> resulting git patches appear to have zero actual content). >> >> http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit >> >> When a developer has to waste several hours rebasing four development >> trees and reworking a large patch series just to address a couple of >> patches that "don't quite look right", it's amazing we get anything >> done. People wonder why their tuners don't get support added sooner? >> THIS IS WHY. >> >> What a colossal waste of my time when I could have been actually >> working on drivers instead... > > As I've explained to you, your patch series were creating and adding into > the kernel building system two files like this one: > > /* > * ngene-av.c: nGene PCIe bridge driver - DVB video/audio support > * > * Copyright (C) 2005-2007 Micronas > * > * Copyright (C) 2008-2009 Ralph Metzler <rjkm@xxxxxxxxxxxxxx> > * Modifications for new nGene firmware, > * support for EEPROM-copying, > * support for new dual DVB-S2 card prototype > * > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License > * version 2 only, as published by the Free Software Foundation. > * > * > * This program is distributed in the hope that it will be useful, > * but WITHOUT ANY WARRANTY; without even the implied warranty of > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > * GNU General Public License for more details. > * > * > * You should have received a copy of the GNU General Public License > * along with this program; if not, write to the Free Software > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > * 02110-1301, USA > * Or, point your browser to http://www.gnu.org/copyleft/gpl.html > */ > > /* This file provides the support functions for DVB audio/video devices > (/dev/dvb/adapter0/[video|audio]), not to be confused with V4L2 support */ > > #include <linux/module.h> > #include <linux/init.h> > #include <linux/delay.h> > #include <linux/slab.h> > #include <linux/poll.h> > #include <linux/io.h> > #include <asm/div64.h> > #include <linux/pci.h> > #include <linux/smp_lock.h> > #include <linux/timer.h> > #include <linux/version.h> > #include <linux/byteorder/generic.h> > #include <linux/firmware.h> > #include <linux/vmalloc.h> > > #include "ngene.h" > > #if 0 > /* some unused code */ > #endif > > > Moving a do-nothing commented code into another file and adding those > /dev/null files into the building system makes no sense at all. It really > doesn't matter if it is just an empty file, or a file with a big #if 0/#endif block. > > The point is that it will add a penalty into the building system and create a new file > without any reason. 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. 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. 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". 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? These arbitrary decisions have real effects on both users and developers - I was going to work on getting the analog support running on that bridge. Instead I spent that time doing work that has *ABSOLUTELY ZERO ACTUAL VALUE* just to make you happy. 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
