Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2
Hello Devin/Mauro, On Fri, May 7, 2010 at 11:52 AM, Mauro Carvalho Chehab mche...@redhat.com wrote: Devin Heitmueller wrote: Hello, Please PULL from http://kernellabs.com/hg/~dheitmueller/ngene2 for the following: Hi Devin, As agreed via IRC with you and stoth, I'm applying all patches, except for the ones that are currently creating unused files at the building system. Let's apply them after you have added some code running there. ngene: properly support boards where channel 0 isn't a TS input ngene: add initial support for digital side of Avermedia m780 ngene: split out i2c code into a separate file Committed. ngene: split out card specific code into a separate file ngene: split out DVB audio/video into a separate file Not merged: it just creates a file with currently unused code. ngene: move more card-specific code to ngene-cards.c This patch were folded with ngene: split out card specific code into a separate file. ngene: split out eeprom functions int a separate file ngene: move a few more DVB a/v related functions Not merged: they just create a file with currently unused code. ngene: start separating out DVB functions into separate file Committed, with a few trivial conflict resolving changes, caused by not merging the 3 previous patches. ngene: Add lgdt3303 and mt2131 deps to Kconfig Committed. I've stored locally the rebased diffs for the non-applied patches. So, on your next ngene pull, you won't need to rebase against my git. It should also be noticed that I needed to apply one patch before your series, in order to fix those compilation errors: drivers/media/dvb/ngene/ngene-cards.c:42:20: error: mt2131.h: Arquivo ou diretório não encontrado drivers/media/dvb/ngene/ngene-cards.c:111: error: variable ‘m780_tunerconfig’ has initializer but incomplete type drivers/media/dvb/ngene/ngene-cards.c:113: warning: excess elements in struct initializer drivers/media/dvb/ngene/ngene-cards.c:113: warning: (near initialization for ‘m780_tunerconfig’) drivers/media/dvb/ngene/ngene-cards.c: In function ‘demod_attach_lg330x’: drivers/media/dvb/ngene/ngene-cards.c:126: error: ‘mt2131_attach’ undeclared (first use in this function) drivers/media/dvb/ngene/ngene-cards.c:126: error: (Each undeclared identifier is reported only once drivers/media/dvb/ngene/ngene-cards.c:126: error: for each function it appears in.) drivers/media/dvb/ngene/ngene-cards.c:126: warning: type defaults to ‘int’ in declaration of ‘__a’ drivers/media/dvb/ngene/ngene-cards.c:126: warning: type defaults to ‘int’ in declaration of ‘type name’ drivers/media/dvb/ngene/ngene-cards.c:126: warning: type defaults to ‘int’ in declaration of ‘type name’ drivers/media/dvb/ngene/ngene-cards.c:126: error: called object ‘__a’ is not a function The fix were to just add the proper tuner include directory: --- a/drivers/media/dvb/ngene/Makefile +++ b/drivers/media/dvb/ngene/Makefile @@ -8,4 +8,4 @@ obj-$(CONFIG_DVB_NGENE) += ngene.o EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ EXTRA_CFLAGS += -Idrivers/media/dvb/frontends/ - +EXTRA_CFLAGS += -Idrivers/media/common/tuners/ Douglas, It seems to me that the better option is if you just pull from ngene2 tree at hg. This will produce a small diff between -git and -hg, but, as according with Devin, they'll send soon an update on ngene driver adding some working code for eeprom and ngene-av, it should probably be ok for you to handle this difference for a couple weeks, especially since it won't produce any runtime difference. Of course, it is up to you to decide, as you're the maintainer of the backport tree ;) I have merged all patches from http://kernellabs.com/hg/~dheitmueller/ngene2. Cheers, Douglas -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2
Devin Heitmueller wrote: Hello, Please PULL from http://kernellabs.com/hg/~dheitmueller/ngene2 for the following: Hi Devin, As agreed via IRC with you and stoth, I'm applying all patches, except for the ones that are currently creating unused files at the building system. Let's apply them after you have added some code running there. ngene: properly support boards where channel 0 isn't a TS input ngene: add initial support for digital side of Avermedia m780 ngene: split out i2c code into a separate file Committed. ngene: split out card specific code into a separate file ngene: split out DVB audio/video into a separate file Not merged: it just creates a file with currently unused code. ngene: move more card-specific code to ngene-cards.c This patch were folded with ngene: split out card specific code into a separate file. ngene: split out eeprom functions int a separate file ngene: move a few more DVB a/v related functions Not merged: they just create a file with currently unused code. ngene: start separating out DVB functions into separate file Committed, with a few trivial conflict resolving changes, caused by not merging the 3 previous patches. ngene: Add lgdt3303 and mt2131 deps to Kconfig Committed. I've stored locally the rebased diffs for the non-applied patches. So, on your next ngene pull, you won't need to rebase against my git. It should also be noticed that I needed to apply one patch before your series, in order to fix those compilation errors: drivers/media/dvb/ngene/ngene-cards.c:42:20: error: mt2131.h: Arquivo ou diretório não encontrado drivers/media/dvb/ngene/ngene-cards.c:111: error: variable ‘m780_tunerconfig’ has initializer but incomplete type drivers/media/dvb/ngene/ngene-cards.c:113: warning: excess elements in struct initializer drivers/media/dvb/ngene/ngene-cards.c:113: warning: (near initialization for ‘m780_tunerconfig’) drivers/media/dvb/ngene/ngene-cards.c: In function ‘demod_attach_lg330x’: drivers/media/dvb/ngene/ngene-cards.c:126: error: ‘mt2131_attach’ undeclared (first use in this function) drivers/media/dvb/ngene/ngene-cards.c:126: error: (Each undeclared identifier is reported only once drivers/media/dvb/ngene/ngene-cards.c:126: error: for each function it appears in.) drivers/media/dvb/ngene/ngene-cards.c:126: warning: type defaults to ‘int’ in declaration of ‘__a’ drivers/media/dvb/ngene/ngene-cards.c:126: warning: type defaults to ‘int’ in declaration of ‘type name’ drivers/media/dvb/ngene/ngene-cards.c:126: warning: type defaults to ‘int’ in declaration of ‘type name’ drivers/media/dvb/ngene/ngene-cards.c:126: error: called object ‘__a’ is not a function The fix were to just add the proper tuner include directory: --- a/drivers/media/dvb/ngene/Makefile +++ b/drivers/media/dvb/ngene/Makefile @@ -8,4 +8,4 @@ obj-$(CONFIG_DVB_NGENE) += ngene.o EXTRA_CFLAGS += -Idrivers/media/dvb/dvb-core/ EXTRA_CFLAGS += -Idrivers/media/dvb/frontends/ - +EXTRA_CFLAGS += -Idrivers/media/common/tuners/ Douglas, It seems to me that the better option is if you just pull from ngene2 tree at hg. This will produce a small diff between -git and -hg, but, as according with Devin, they'll send soon an update on ngene driver adding some working code for eeprom and ngene-av, it should probably be ok for you to handle this difference for a couple weeks, especially since it won't produce any runtime difference. Of course, it is up to you to decide, as you're the maintainer of the backport tree ;) -- Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
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... 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
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). That's the reason why I did not move the the unused code to separate files. I wanted to keep it in the HG repository. I hope that someone will remember that there is additional code which can be retrieved from the initial check-in. Otherwise, developers will waste their time to reinvent the the wheel. CU Oliver -- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
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 r...@metzlerbros.de * 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. -- Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
On Tue, Mar 23, 2010 at 9:45 AM, Mauro Carvalho Chehab mche...@redhat.com 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 r...@metzlerbros.de * 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
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
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. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
On Tue, Mar 23, 2010 at 10:48 AM, Mauro Carvalho Chehab mche...@redhat.com 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
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. Mauro, It makes no sense to have Kernel Labs work out of tree. Clearly in some cases this makes a lot of short term sense but rarely does this scale long term. I can speak to this from first hand experience. Generally it leads to bit-rot and unhappy developers, unhappy community and unhappy users. We are cleanup up the existing tree and fixing major oops prior to a massive amount of analog work. We've spent the time partitioning the code (like all of the other drivers) into sub-units -video.c, -dvb.c as this simplifies peer-to-peer remote development. It breeds commonality between drivers and eases long-term support for developers familiar with other drivers. The end goal is to show a clear and concise series of patches showing migration from the driver as-is to something much more stable and commercial grade. We can demonstrate a clean implementation and have near-zero development conflicts. I see no failure on our part. I see this as positive and pro-active peer-to-peer remote development practices. In fairness to you, I also understand your comments and they are also valid concerns. We are mindful of your authority and aware of your needs to keep the tree clean and optimal. On balance I think we both want the same end goal. Good code, clean patches, understandable small changes showing slow evolution. I hope you can see a way forward for the ngene tree that doesn't mean we are forced to house it away from linuxtv.org. This would be a great pity and against the spirit of community development. Best, - Steve -- Steven Toth - Kernel Labs http://www.kernellabs.com +1.646.355.8490 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
Steven Toth wrote: 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. Mauro, It makes no sense to have Kernel Labs work out of tree. Clearly in some cases this makes a lot of short term sense but rarely does this scale long term. I can speak to this from first hand experience. Generally it leads to bit-rot and unhappy developers, unhappy community and unhappy users. We are cleanup up the existing tree and fixing major oops prior to a massive amount of analog work. We've spent the time partitioning the code (like all of the other drivers) into sub-units -video.c, -dvb.c as this simplifies peer-to-peer remote development. It breeds commonality between drivers and eases long-term support for developers familiar with other drivers. The end goal is to show a clear and concise series of patches showing migration from the driver as-is to something much more stable and commercial grade. We can demonstrate a clean implementation and have near-zero development conflicts. I see no failure on our part. I see this as positive and pro-active peer-to-peer remote development practices. I agree with this concept. Breaking a big file into logically related separate files makes sense. Yet, Devin's commented on irc that the code on those files (ngene-av and ngene-eeprom) are from some hardware that you're not working, nor have any plans to deal with them currently. So, the patches would be just adding some dead code into some files that may never be touched. That's why I said him that the better is to just drop it. The original code is at -hg tree, and, people can use it any time it would be needed. I did something like that in December, when I've added ISDB-T support into Siano driver: I got the history of siano include files, restored the ISDB-T structures and wrote the code for the ISDB-T stats, based on it. In fairness to you, I also understand your comments and they are also valid concerns. We are mindful of your authority and aware of your needs to keep the tree clean and optimal. On balance I think we both want the same end goal. Good code, clean patches, understandable small changes showing slow evolution. I hope you can see a way forward for the ngene tree that doesn't mean we are forced to house it away from linuxtv.org. This would be a great pity and against the spirit of community development. That's said, if I understood Devin wrong or if you now have plans to add some real code at the new ngene-av and ngene-eeprom files, that's fine for me. I'll happily accept a patch that moves that code to another file and enable the code to read eeprom and to use the AV part, even if you do it on separate patches, inside the same pull request. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://kernellabs.com/hg/~dheitmueller/ngene2-bullshit
Mauro Carvalho Chehab wrote: Steven Toth wrote: 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. Mauro, It makes no sense to have Kernel Labs work out of tree. Clearly in some cases this makes a lot of short term sense but rarely does this scale long term. I can speak to this from first hand experience. Generally it leads to bit-rot and unhappy developers, unhappy community and unhappy users. We are cleanup up the existing tree and fixing major oops prior to a massive amount of analog work. We've spent the time partitioning the code (like all of the other drivers) into sub-units -video.c, -dvb.c as this simplifies peer-to-peer remote development. It breeds commonality between drivers and eases long-term support for developers familiar with other drivers. The end goal is to show a clear and concise series of patches showing migration from the driver as-is to something much more stable and commercial grade. We can demonstrate a clean implementation and have near-zero development conflicts. I see no failure on our part. I see this as positive and pro-active peer-to-peer remote development practices. I agree with this concept. Breaking a big file into logically related separate files makes sense. Yet, Devin's commented on irc that the code on those files (ngene-av and ngene-eeprom) are from some hardware that you're not working, nor have any plans to deal with them currently. So, the patches would be just adding some dead code into some files that may never be touched. That's why I said him that the better is to just drop it. The original code is at -hg tree, and, people can use it any time it would be needed. I did something like that in December, when I've added ISDB-T support into Siano driver: I got the history of siano include files, restored the ISDB-T structures and wrote the code for the ISDB-T stats, based on it. In fairness to you, I also understand your comments and they are also valid concerns. We are mindful of your authority and aware of your needs to keep the tree clean and optimal. On balance I think we both want the same end goal. Good code, clean patches, understandable small changes showing slow evolution. I hope you can see a way forward for the ngene tree that doesn't mean we are forced to house it away from linuxtv.org. This would be a great pity and against the spirit of community development. That's said, if I understood Devin wrong or if you now have plans to add some real code at the new ngene-av and ngene-eeprom files, that's fine for me. I'll happily accept a patch that moves that code to another file and enable the code to read eeprom and to use the AV part, even if you do it on separate patches, inside the same pull request. Hmm... this were badly written, partially due to a typo. So let me re-phrase it: If my understanding from Devin's chat were wrong and if you actually have plans to add some working code at the new ngene-av and ngene-eeprom files, that's fine for me. I'll happily accept a patch that moves that code to another file and enable the code to read eeprom and to use the AV part, even if you do it on separate patches, inside the same pull request. -- Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
