Re: [PATCH 1/5] rc-code: merge and rename ir-core
On Wed, 2010-09-08 at 23:12 +0200, David Härdeman wrote: On Wed, Sep 08, 2010 at 11:10:40AM -0400, Andy Walls wrote: Tag files and a decent editor are all one needs for full code navigation. The kernel makefile already has a tags target to make the tags file. If you like to use tags, it won't be affected by many or few files so it's not an argument for either approach. Smaller files make for better logical isolation of functions,limiting visibilty/scope, Limiting visibility so that you'll have to add explicit declarations to ir-core-priv.h for inter-file function calls and functions that would otherwise be unnecessary (ir_raw_get_allowed_protocols() for example) doesn't sound like an advantage to me. and faster compilation of a file (but maybe at the expense of link time). *sigh* compile times on my laptop: rc-core.o 0.558s ir-functions.o0.321s ir-sysfs.o0.251s ir-raw-event.o0.397s rc-map.o 0.265s I personally don't care much about this. I use Kscope all the time :-) However I do care and like very much this patchset because I also would like to have a per remote input device, and this is first step towards it. Best regards, Maxim Levitsky -- 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: [PATCH 1/5] rc-code: merge and rename ir-core
Em 07-09-2010 18:51, David Härdeman escreveu: This patch merges the files which makes up ir-core and renames the resulting module to rc-core. IMHO this makes it much easier to hack on the core module since all code is in one file. This also allows some simplification of ir-core-priv.h as fewer internal functions need to be exposed. I'm not sure about this patch. Big files tend to be harder to maintain, as it takes more time to find the right functions inside it. Also, IMO, it makes sense to keep the raw-event code on a separate file. Anyway, if we apply this patch right now, it will cause merge conflicts with the input tree, due to the get/setkeycodebig patches, and with some other patches that are pending merge/review. The better is to apply such patch just after the release of 2.6.37-rc1, after having all those conflicts solved. 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: [PATCH 1/5] rc-code: merge and rename ir-core
On Wed, Sep 08, 2010 at 10:42:10AM -0300, Mauro Carvalho Chehab wrote: Em 07-09-2010 18:51, David Härdeman escreveu: This patch merges the files which makes up ir-core and renames the resulting module to rc-core. IMHO this makes it much easier to hack on the core module since all code is in one file. This also allows some simplification of ir-core-priv.h as fewer internal functions need to be exposed. I'm not sure about this patch. Big files tend to be harder to maintain, as it takes more time to find the right functions inside it. Also, IMO, it makes sense to keep the raw-event code on a separate file. There's definitely a balance to be struck between file size and file count. Having all the relevant code in one file definitely has its advantage in that its easier to jump around from function to function and trace code paths taken, but I can see the argument for isolating the raw event handling code a bit too, especially if its going to be further expanded, which I believe is likely the case. So I guess I'm on the fence here. :) Anyway, if we apply this patch right now, it will cause merge conflicts with the input tree, due to the get/setkeycodebig patches, and with some other patches that are pending merge/review. The better is to apply such patch just after the release of 2.6.37-rc1, after having all those conflicts solved. The imon patch that moves mouse/panel/knob input to its own input device should be possible to take in advance of everything else, more or less, though I need to finish actually testing it out (and should probably make some further imon fixes for issues listed in a kernel.org bugzilla, the number of which escapes me at the moment). -- Jarod Wilson ja...@redhat.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: [PATCH 1/5] rc-code: merge and rename ir-core
Tag files and a decent editor are all one needs for full code navigation. The
kernel makefile already has a tags target to make the tags file.
Smaller files make for better logical isolation of functions, limiting
visibilty/scope, and faster compilation of a file (but maybe at the expense of
link time). That sort of isolation of functionality into smaller files also
makes the code more digestable for someone new looking at it, IMO.
Large files are good for a pile of stuff no one ever needs to look at again:
well tested code that has an expected low rate of change in the future.
Regards,
Andy
Jarod Wilson ja...@redhat.com wrote:
On Wed, Sep 08, 2010 at 10:42:10AM -0300, Mauro Carvalho Chehab wrote:
Em 07-09-2010 18:51, David Härdeman escreveu:
This patch merges the files which makes up ir-core and renames the
resulting module to rc-core. IMHO this makes it much easier to hack
on the core module since all code is in one file.
This also allows some simplification of ir-core-priv.h as fewer internal
functions need to be exposed.
I'm not sure about this patch. Big files tend to be harder to maintain,
as it takes more time to find the right functions inside it. Also, IMO,
it makes sense to keep the raw-event code on a separate file.
There's definitely a balance to be struck between file size and file
count. Having all the relevant code in one file definitely has its
advantage in that its easier to jump around from function to function and
trace code paths taken, but I can see the argument for isolating the raw
event handling code a bit too, especially if its going to be further
expanded, which I believe is likely the case. So I guess I'm on the
fence here. :)
Anyway, if we apply this patch right now, it will cause merge conflicts with
the input tree, due to the get/setkeycodebig patches, and with some other
patches that are pending merge/review. The better is to apply such patch
just after the release of 2.6.37-rc1, after having all those conflicts
solved.
The imon patch that moves mouse/panel/knob input to its own input device
should be possible to take in advance of everything else, more or less,
though I need to finish actually testing it out (and should probably make
some further imon fixes for issues listed in a kernel.org bugzilla, the
number of which escapes me at the moment).
--
Jarod Wilson
ja...@redhat.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
N�r��yb�X��ǧv�^�){.n�+{���bj)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
Re: [PATCH 1/5] rc-code: merge and rename ir-core
On Wed, Sep 08, 2010 at 11:10:40AM -0400, Andy Walls wrote: Tag files and a decent editor are all one needs for full code navigation. The kernel makefile already has a tags target to make the tags file. If you like to use tags, it won't be affected by many or few files so it's not an argument for either approach. Smaller files make for better logical isolation of functions,limiting visibilty/scope, Limiting visibility so that you'll have to add explicit declarations to ir-core-priv.h for inter-file function calls and functions that would otherwise be unnecessary (ir_raw_get_allowed_protocols() for example) doesn't sound like an advantage to me. and faster compilation of a file (but maybe at the expense of link time). *sigh* compile times on my laptop: rc-core.o 0.558s ir-functions.o 0.321s ir-sysfs.o 0.251s ir-raw-event.o 0.397s rc-map.o0.265s That sort of isolation of functionality into smaller files also makes the code more digestable for someone new looking at it, IMO. First of all, I personally find it much easier to grok a new subsystem if the relevant parts are gathered into one file, both when I'm new to a subsystem and when I'm used to it. drivers/input/input.c and drivers/input/evdev.c come to mind as good examples. But more importantly, how about focusing on the people actually writing patches for ir-core rather than hypotetical people? -- David Härdeman -- 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: [PATCH 1/5] rc-code: merge and rename ir-core
On Wed, Sep 08, 2010 at 10:42:10AM -0300, Mauro Carvalho Chehab wrote: Em 07-09-2010 18:51, David Härdeman escreveu: This patch merges the files which makes up ir-core and renames the resulting module to rc-core. IMHO this makes it much easier to hack on the core module since all code is in one file. This also allows some simplification of ir-core-priv.h as fewer internal functions need to be exposed. I'm not sure about this patch. Big files tend to be harder to maintain, as it takes more time to find the right functions inside it. Also, IMO, it makes sense to keep the raw-event code on a separate file. I don't find big files difficult (note: we're talking about 1300 lines here). Rather the opposite, no hesitation about which files a given function originates from and all related code in one nice file. evdev.c and input.c are good precedents. But of course, it all boils down to a matter of personal taste. Anyway, if we apply this patch right now, it will cause merge conflicts with the input tree, due to the get/setkeycodebig patches, and with some other patches that are pending merge/review. The better is to apply such patch just after the release of 2.6.37-rc1, after having all those conflicts solved. I agree that the big scancode patches from the input tree should go first. I keep updating my patchset as the media_tree (staging/v2.6.37 branch) changes so I have no problem sending an updated patchset at a suitable time in the future. -- David Härdeman -- 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: [PATCH 1/5] rc-code: merge and rename ir-core
On Wed, Sep 8, 2010 at 5:42 PM, David Härdeman da...@hardeman.nu wrote: On Wed, Sep 08, 2010 at 10:42:10AM -0300, Mauro Carvalho Chehab wrote: Em 07-09-2010 18:51, David Härdeman escreveu: This patch merges the files which makes up ir-core and renames the resulting module to rc-core. IMHO this makes it much easier to hack on the core module since all code is in one file. This also allows some simplification of ir-core-priv.h as fewer internal functions need to be exposed. I'm not sure about this patch. Big files tend to be harder to maintain, as it takes more time to find the right functions inside it. Also, IMO, it makes sense to keep the raw-event code on a separate file. I don't find big files difficult (note: we're talking about 1300 lines here). Rather the opposite, no hesitation about which files a given function originates from and all related code in one nice file. evdev.c and input.c are good precedents. But of course, it all boils down to a matter of personal taste. I think I have to finally admit that my personal taste tends toward one big file in this particular case. Anyway, if we apply this patch right now, it will cause merge conflicts with the input tree, due to the get/setkeycodebig patches, and with some other patches that are pending merge/review. The better is to apply such patch just after the release of 2.6.37-rc1, after having all those conflicts solved. I agree that the big scancode patches from the input tree should go first. I keep updating my patchset as the media_tree (staging/v2.6.37 branch) changes so I have no problem sending an updated patchset at a suitable time in the future. Please feel free to add this to the subsequent refreshes: Acked-by: Jarod Wilson ja...@redhat.com -- Jarod Wilson ja...@wilsonet.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
