Development: Hints for Refactoring Existing Drivers: Difference between revisions
No edit summary |
No edit summary |
||
Line 4: | Line 4: | ||
You don't need to be core-developer to do so, everybody is invited to review the code, join development and contribute. |
You don't need to be core-developer to do so, everybody is invited to review the code, join development and contribute. |
||
Basically [[Code Review]] can be done on very different levels: just check the code for correctness and perform [[Bugfix]]es, introduce [[Performance Optimisation]]s or, '''Refactoring''. |
Basically [[Code Review]] can be done on very different levels: just check the code for correctness and perform [[Bugfix]]es, introduce [[Performance Optimisation]]s or, '''Refactoring'''. |
||
Revision as of 14:51, 27 September 2004
Introduction
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. You don't need to be core-developer to do so, everybody is invited to review the code, join development and contribute.
Basically Code Review can be done on very different levels: just check the code for correctness and perform Bugfixes, introduce Performance Optimisations or, Refactoring.
What is Refactoring?
You find a short definition of the Refactoring Process on Wikipedia, mentioning the important facts. The basic principles 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.
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.
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.
Refactoring does not means 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 Great to learn performant and compact Coding.
So please: Whatever you do, please keep it Simple.
Please do never ever apply big Changes in-place
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.
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.