Development: Hints for Refactoring Existing Drivers: Difference between revisions

From LinuxTVWiki
Jump to navigation Jump to search
m (fixed links,)
 
(9 intermediate revisions by 2 users not shown)
Line 1: Line 1:
Free software often grows over time -- both in terms of the amount of code and as well as the number of individuals contributing to the code base. In fact, with free and open source software, over the course of the project's development life, it is quite common for other people to become involved simply because the original author(s) may not always be available or even willing to maintain the code forever. This holds true for V4L-DVB drivers and associative utilities. Yet, in our case, we'd like to stress that you don't need to be a core developer in order to become involved in our software's continued progression, as everybody is invited to [[Development: Code Review|review the code]] and contribute. All in all, contributions can be done on several different levels that vary in complexity; for example:
Free software grows over time, many people are involved in development and not always the original authors are still available or willing to maintain the code forever. So then and when the code has to get ''cleaned up'', ''Refactored'' to make it future-proof.
* [[Development: How to submit patches|patches]] to the code for the purposes of providing
You don't need to be core-developer to do so, everybody is invited to review the code, join development and contribute.
** simple clean-ups (such as remedying spelling/typo/grammar/punctuation errors or coding style violations)

** more detailed bug fixes and/or logical corrections
Basically [[Development: Code Review|Code Review]] can be done on very different levels: just check the code for correctness and perform [[Bugfix]]es, introduce [[Development: DVB Driver Performance Optimisations]] or, '''Refactoring'''.
* more involved changes that introduce [[Development: Driver Performance Optimisations|driver performance optimisations]]

* introduction of new feature sets to the guiding [[Development:_Linux_Media_Infrastructure_API|Linux Media Infrastructure API]]
* a complete '''refactoring''' of portions of the V4L-DVB code base, typically done in light of simplifying the code, and perhaps making it more future-proof


== What is ''Refactoring''? ==
== What is ''Refactoring''? ==


You find a short definition of [[Wikipedia:Refactoring|the Refactoring Process]] on Wikipedia, mentioning the important facts. The basic principles are:
Wikipedia provides [[Wikipedia:Refactoring|a brief definition and discussion of the refactoring process]], in which you will find mentioned that the basic principles espoused are:


* ''Simplify your Code.''
* ''Simplify your Code.''
Line 25: Line 27:
* ''Simplify your Code''.
* ''Simplify your Code''.


Refacturing is a natural process, usually projects grow over time and then and when this naturally grown jungle needs to get urbanized again to make it a nice place to live. This process is usually accompanied by (sometimes loud and rude) discussions about [[Philosophy and holy Wars]], but don't worry: dogs that make noise usually don't bite. Revolutionary ideas sometimes need some fighting to get through.
Refactoring is a natural process in respect to projects that usually grow over time. To use an analogy: every now and then, what has grown through natural means into a jungle will need to be cut back in order to make it a nice place to live again. Unfortunately, often due to clashes of vanities and feelings of possessiveness, the steps towards a refactoring can sometimes be accompanied by loud discussions that equate to little more than unruly philosophical debates and holy wars. But don't worry: dogs that make noise usually don't bite. Revolutionary ideas sometimes need some fighting to get through.


Since with time developers get a little blind and used to their own weaknesses it makes sense that reviewer/refactorer and programmer are not the same person. No part of the Linux kernel is exclusively owned by a maintainer. If a new solution makes sense it will make it sooner or later into the public source tree.
Since developers may become, with time, a little myopic in respect towards their own code, it makes sense that the reviewer/refactorer and programmer are often not the same person. No part of the Linux kernel is exclusively owned by a maintainer. If a new solution makes sense, sooner or later it will make it into the public source tree.

Refactoring does not means new features. Refactoring means that you make the code maintainable.


Refactoring does not mean new features. Refactoring means that you make the code maintainable.


== Keep it Simple ==
== Keep it Simple ==


The simpler the resulting code is the easier it will be to maintain. Even from the performance point of view simple code is almost always preferable.
The simpler the resulting code is, the easier it will be to maintain. Even from the performance point of view, simple code is almost always preferable.


If you want to see some impressive code following this approach you may want to check out one of Fabrice Bellards Projects, e.g. [http://fabrice.bellard.free.fr/TinyGL/ TinyGL] or the ID software GPL games [http://www.idsoftware.com/business/techdownloads/ Doom, Quake & Co] Great to learn performant and compact Coding.
If you want to see some impressive code following this approach, you may want to check out one of Fabrice Bellards Projects, e.g. [http://fabrice.bellard.free.fr/TinyGL/ TinyGL] or the ID software GPL games [http://www.idsoftware.com/business/techdownloads/ Doom, Quake & Co] -- both of these are great examples of highly efficient coding (in terms of both compactness and performance).


So please: '''Whatever you do, please keep it Simple.'''
So please: '''Whatever you do, please keep it Simple.'''


== Please Never Ever Try to Apply Big Changes All in One Shot ==


If you plan to rewrite bigger portions of the code please don't create a huge patch or thousands of patches.
== Please do never ever apply big Changes in-place ==
Please avoid postings like ''"[PATCH] I ported all drivers to this or that kernel API because this is the way the kernel maintainers like it to see!"''. Instead, fork the relevant code modules into a separate development repo so that people can easily test them concurrently with the old code and are free to continue to use the old, usually well-tested, code until your new version has matured.


That practice wasn't always adhered to during the history of V4L-DVB code development and, unfortunately, there were several case or situations during which some elements of the code base were barely usable for weeks or even months because of neglecting the principle laid out above.
If you plan to rewrite bigger portions of the code please don't create a huge patch or thousands of patches but fork the relevant code modules so that people can easily test them concurrently with the old code and are free to use the old, usually well-tested code until your new version has matured.


The bottom line is that you should respect that other people are using V4L-DVB software in productive environments and may rely upon a working code base. In addition, one may never be able to foresee the effect of major changes on all the exotic platforms the V4L-DVB stack is used upon. If you really think using new APIs is the way to go then please let's fork the source tree and move over to the new tree as soon it is well-tested and verified upon all architectures.
There have been several situations in the past where the linux-dvb CVS was barely usable for weeks or months because we did not followed this principle. Please respect that other people are using the linux-dvb source in productive environments and rely on a working code base.

Please avoid postings like ''"[PATCH] I ported all drivers to this and that kernel API because this is the way the kernel maintainers like it to see!"''. Sometimes people are going their own way with reason, you are never able to foresee the effect of such major changes on all the exotic platforms the linux-dvb stack is used. If you really think using the new API is the way to go then please let's fork the source tree and move over to the new tree as soon it is verified on all architectures and well-tested.


== Psychological Prerequisites ==

'''Refactoring''' does not only means to rewrite code but also to convince all other developers to accept the changes and to join your efforts.


== Addendum: The Psychological Prerequisites for Refactoring ==
'''Refactoring''' is a complex process and does not simply mean rewriting the existing code. Rather, it also involves trying to convince other developers to accept your changes, and, it is this later point that is perhaps the much more difficult part of the process. We encourage you to rise up to that challenge and present two such approaches you might take in that regard:


=== Approach #1: The Democratic Trial -- let's discuss! ===
=== Approach #1: The Democratic Trial -- let's discuss! ===


* Somebody starts the discussion and makes his prosaic proposal.
* Somebody starts the discussion and makes his or her prosaic proposal.


* nothing happens for a while.
* nothing happens for a while.
Line 92: Line 90:


* Sometimes a few mails refining the final details close the discussion.
* Sometimes a few mails refining the final details close the discussion.



=== Approach #2: Just do it ===
=== Approach #2: Just do it ===
The process outlined above may sound very democratic and entertaining, but it eats up a lot of time and can seriously hamper good spirits/mood, and, thus, can be kind of counterproductive. If you don't want to spend your energy in those regards, or risk being sucked into flame wars, you may want to try this:


* Implement your approach for 2 or 3 sample drivers -- perhaps showcasing your changes with a complex driver and as well with two more simpler cases
The process outlined above may sound very democratic and entertaining but it eats a lot of time and good mood, is thus kind of counterproductive. If you don't want to spend your energy in flame wars you may try this:
* Then post your code changes on the [mailto:majordomo@vger.kernel.org?body=subscribe%20linux-media Linux-Media Mailing List] and propose to start a new development branch where this work can be further continued across the entire V4L-DVB code base. If you don't already have a development repo with the V4L-DVB project from which you can work, one can easily be provided if you request (though this is certainly not a requirement).

* Implement your approach for 2 or 3 sample drivers (choose a complex one, e.g. a STB driver or the av711x-ttpci driver and 2 simple ones, a hotplug-USB device and a modern PCI card).


If your code revisions have genuine, and easily provable, advantages over the older code, it is highly likely that your implementation will be approved of and, ultimately, incorporated within the V4L-DVB source.
* Post your code on the linux-dvb mailing list and propose to start a new development branch in CVS where this work can get continued. Request a CVS account if you don't already have one so that you can easily work on your code.


If your new code has serious advantages it will get incorporated for sure. In most cases it will take less time to write the new code than to await the end of discussion.
While this approach runs somewhat in opposition to the "Democratic Discussion Process", like that outlined in the prior section, it will, however, in most cases, take a whole lot less time to just write the new code and present it, as described above, then having to await the end of any lengthy discussions.


In addition, this latter approach may be able to steer you, and the community at large, clear away from any sort of nefarious activities that may prevent the emergence of homogeneous and easy-to-maintain code. You are dealing with fellow human beings and, unfortunately, you can not account for all of their behaviour to be gracious or dignified. The democratic approach introduces the risk that discussions will devolve into harsh words being used as egos may be hurt due to one code base being rejected or replaced by another. Such occurrences can then spawn machinations within some individuals -- they may later try to "personalize" the code base just because they can't resist putting in some pieces of their old theory just to keep it alive, and nobody within the community asks why they're doing it simply because everybody fears or is un-interested in becoming involved in yet another discussion with that person which is bound to degenerate into a new flame war.
DVB driver development is done by everybody who wants to join development, no Guru has absolute power over the code. So you can be sure that everything that makes sense will make it sooner or later into the mainstream source tree.


On a much more positive note, there may be a beneficial side effect for the development community at large from your having adopted the approach outlined within this section. Specifically, any rational developer will be highly motivated to make their own code as elegant as your own reference code.
Additionally, you achieved a nice side effect: Every developer will be highly motivated to make his own driver as elegant as your reference code, so you can be sure that all developers will join the development. This is in opposition to the "Democratic Discussion Process" where the risk is high that first many developers are pissed off by harsh words in discussion and later try to "personalize" the code just because they can't resist to put in some pieces of their old theory just to keep it alive. And nobody asks why they're doing it because everybody fears a new flame war (what again may prevent emergence of homogenous and easy-to-maintain code).


Lastly, bear in mind that the V4L-DVB project is contributed to by anyone who wants to join development, and that no one person or guru, so to speak, has absolute power over the code. So take heart in the fact that everything that makes sense will sooner or later make it into the mainstream V4L-DVB source tree.


[[Category:Development]]
[[Category:Development]]

Latest revision as of 05:22, 8 November 2009

Free software often grows over time -- both in terms of the amount of code and as well as the number of individuals contributing to the code base. In fact, with free and open source software, over the course of the project's development life, it is quite common for other people to become involved simply because the original author(s) may not always be available or even willing to maintain the code forever. This holds true for V4L-DVB drivers and associative utilities. Yet, in our case, we'd like to stress that you don't need to be a core developer in order to become involved in our software's continued progression, as everybody is invited to review the code and contribute. All in all, contributions can be done on several different levels that vary in complexity; for example:

  • patches to the code for the purposes of providing
    • simple clean-ups (such as remedying spelling/typo/grammar/punctuation errors or coding style violations)
    • more detailed bug fixes and/or logical corrections
  • more involved changes that introduce driver performance optimisations
  • introduction of new feature sets to the guiding Linux Media Infrastructure API
  • a complete refactoring of portions of the V4L-DVB code base, typically done in light of simplifying the code, and perhaps making it more future-proof

What is Refactoring?

Wikipedia provides a brief definition and discussion of the refactoring process, in which you will find mentioned that the basic principles espoused are:

  • Simplify your Code.
  • Don't introduce new Features in the Refactoring Process.
  • Break down API Barriers where they don't make Sense
  • Introduce new Interfaces where they stabilize your Code.
  • Try to keep external Interfaces as stable as possible.
  • Split complex Modules into smaller, easily testable ones with well-defined Tasks.
  • Avoid Confusion.
  • Simplify your Code.

Refactoring is a natural process in respect to projects that usually grow over time. To use an analogy: every now and then, what has grown through natural means into a jungle will need to be cut back in order to make it a nice place to live again. Unfortunately, often due to clashes of vanities and feelings of possessiveness, the steps towards a refactoring can sometimes be accompanied by loud discussions that equate to little more than unruly philosophical debates and holy wars. But don't worry: dogs that make noise usually don't bite. Revolutionary ideas sometimes need some fighting to get through.

Since developers may become, with time, a little myopic in respect towards their own code, it makes sense that the reviewer/refactorer and programmer are often not the same person. No part of the Linux kernel is exclusively owned by a maintainer. If a new solution makes sense, sooner or later it will make it into the public source tree.

Refactoring does not mean new features. Refactoring means that you make the code maintainable.

Keep it Simple

The simpler the resulting code is, the easier it will be to maintain. Even from the performance point of view, simple code is almost always preferable.

If you want to see some impressive code following this approach, you may want to check out one of Fabrice Bellards Projects, e.g. TinyGL or the ID software GPL games Doom, Quake & Co -- both of these are great examples of highly efficient coding (in terms of both compactness and performance).

So please: Whatever you do, please keep it Simple.

Please Never Ever Try to Apply Big Changes All in One Shot

If you plan to rewrite bigger portions of the code please don't create a huge patch or thousands of patches. Please avoid postings like "[PATCH] I ported all drivers to this or that kernel API because this is the way the kernel maintainers like it to see!". Instead, fork the relevant code modules into a separate development repo so that people can easily test them concurrently with the old code and are free to continue to use the old, usually well-tested, code until your new version has matured.

That practice wasn't always adhered to during the history of V4L-DVB code development and, unfortunately, there were several case or situations during which some elements of the code base were barely usable for weeks or even months because of neglecting the principle laid out above.

The bottom line is that you should respect that other people are using V4L-DVB software in productive environments and may rely upon a working code base. In addition, one may never be able to foresee the effect of major changes on all the exotic platforms the V4L-DVB stack is used upon. If you really think using new APIs is the way to go then please let's fork the source tree and move over to the new tree as soon it is well-tested and verified upon all architectures.

Addendum: The Psychological Prerequisites for Refactoring

Refactoring is a complex process and does not simply mean rewriting the existing code. Rather, it also involves trying to convince other developers to accept your changes, and, it is this later point that is perhaps the much more difficult part of the process. We encourage you to rise up to that challenge and present two such approaches you might take in that regard:

Approach #1: The Democratic Trial -- let's discuss!

  • Somebody starts the discussion and makes his or her prosaic proposal.
  • nothing happens for a while.
  • Guru #1 says: "mmhh... interesting!"
  • Guru #2 says: "No go!!! we need to do it like I already started years ago!!"
  • Guru #3 says: "yes, Guru #2 is right, we have to follow the mainstream API development!"
  • Guru #1 annotates that the new approach has some appealing properties and simplifies many things a lot.
  • several less involved developers join discussion and take sides with Guru #1, #2 or Guru #3.
  • The Proposer refines his approach and maybe even provides some sample code.
  • Guru #1 says that he likes it.
  • Guru #2 or Guru #3 make their own proposal and post some sample code explaining the plan they are following since years.
  • Guru #1 calls it bloat and emphasizes how simple the new approach may be.
  • Guru #3 says that we would have to rewrite a lot of code.
  • (mails get harsh in sound, sometimes even quite personal. Arguments count less and less.)
  • The Proposer, Guru #2 and Guru #3 post several new proposals converging closer to each other. The sample code gets more and more complicated due to all the special cases that come to discussion.
  • For a while it becomes quiet.
  • Guru #2 or Guru #3 posts a new approach that funnily looks quite similar in concept to the original code of the proposer but is a little less clean due to the exhausting fights.
  • Proposer says: "hey, that looks familiar!" -- Guru: "may be."
  • Sometimes a few mails refining the final details close the discussion.

Approach #2: Just do it

The process outlined above may sound very democratic and entertaining, but it eats up a lot of time and can seriously hamper good spirits/mood, and, thus, can be kind of counterproductive. If you don't want to spend your energy in those regards, or risk being sucked into flame wars, you may want to try this:

  • Implement your approach for 2 or 3 sample drivers -- perhaps showcasing your changes with a complex driver and as well with two more simpler cases
  • Then post your code changes on the Linux-Media Mailing List and propose to start a new development branch where this work can be further continued across the entire V4L-DVB code base. If you don't already have a development repo with the V4L-DVB project from which you can work, one can easily be provided if you request (though this is certainly not a requirement).

If your code revisions have genuine, and easily provable, advantages over the older code, it is highly likely that your implementation will be approved of and, ultimately, incorporated within the V4L-DVB source.

While this approach runs somewhat in opposition to the "Democratic Discussion Process", like that outlined in the prior section, it will, however, in most cases, take a whole lot less time to just write the new code and present it, as described above, then having to await the end of any lengthy discussions.

In addition, this latter approach may be able to steer you, and the community at large, clear away from any sort of nefarious activities that may prevent the emergence of homogeneous and easy-to-maintain code. You are dealing with fellow human beings and, unfortunately, you can not account for all of their behaviour to be gracious or dignified. The democratic approach introduces the risk that discussions will devolve into harsh words being used as egos may be hurt due to one code base being rejected or replaced by another. Such occurrences can then spawn machinations within some individuals -- they may later try to "personalize" the code base just because they can't resist putting in some pieces of their old theory just to keep it alive, and nobody within the community asks why they're doing it simply because everybody fears or is un-interested in becoming involved in yet another discussion with that person which is bound to degenerate into a new flame war.

On a much more positive note, there may be a beneficial side effect for the development community at large from your having adopted the approach outlined within this section. Specifically, any rational developer will be highly motivated to make their own code as elegant as your own reference code.

Lastly, bear in mind that the V4L-DVB project is contributed to by anyone who wants to join development, and that no one person or guru, so to speak, has absolute power over the code. So take heart in the fact that everything that makes sense will sooner or later make it into the mainstream V4L-DVB source tree.