-----Original Message----- From: ros-dev-bounces@reactos.org [mailto:ros-dev-bounces@reactos.org] On Behalf Of Alex Ionescu Sent: 19. oktober 2005 17:08 To: ReactOS Development List Subject: Re: [ros-dev] top-level source header
Casper Hornstrup wrote:
You can lookup the information in the PROGRAMMERS field in the repository. The PURPOSE field is not needed as you should be able to get the same information from the filename and the code in the file.
I agree it can be considered superflous, but having such identification information is usually considered good practice in most software projects. While -we- can lookup that information, a non-programmer will have NO clue what on Earth a file does simply by browsing through it.
[CSH] I'm wondering why it is considered good practice. If a non-programmer has no clue as to what the file contents does from reading the source code, why does he need to know the information contained in a one line summary description of the file contents?
As
for the PROGRAMMERS information, it can be much more useful and correct then looking up anyone that ever touched the file.
[CSH] Well history has proved that statement wrong. We have had and still have files with headers which were just copied from another file and not changed, so it has the wrong information in it. I'm sure you have seen that when you worked on ntoskrnl. The repository has correct information when the committer was the author and when he isn't, it is usually practised that the original author is mentioned in the commit message.
It can also provide
email information, which SVN doesn't.
[CSH] People change e-mail addresses all the time. During this project, I've had at least 5 difference e-mail addresses. Repository usernames don't change. When you change e-mail address, do you run through all the headers and change it? I'm sure many people wouldn't.
We can maintain e-mail addresses at: http://www.reactos.org/wiki/index.php/Committers then you only need to change it in one central place.
An example: if Foo is browing our
sources to create some documentation, I'm sure he'd prefer knowing the purpose of each file right out of the bat, and also have contact information for the main developer(s) so he can ask questions.
[CSH] Tell him to browse using ViewCVS. He can get the username from there. If he is too lazy to look up the e-mail address of the committer, then we can create a website that does it for him. Still way much less work than maintaining that information on 10.000+ files in the future. And when you re-arrange the structure of the code (like for instance your ntoskrnl changes), the purpose of the files may change and you need to update the purpose descriptions. Extra work when developing and history tells us that developers forget about such things possible because the compiler doesn't warn about it.
If you can't see the purpose from this, then the code isn't as clear as it could be.
You're assuming everyone is a developer. You're also assuming every comments their code. There's some stuff in ntoskrnl\mm that I can barely begin to understand; I don't want to imagine what a non-kernel dev, or even a non-dev would glean from it.
[CSH] You don't need comments to make code understandable. Maybe that code could need some refactoring? What I would really like to know now is, why is a non-programmer reading source code for a piece of highly complex software like ReactOS?
It's kind of like documenting your way out of unreadable code.
Microsoft extensively comments their files yet also includes a "Purpose" field. Almost every software project in the world does.
[CSH] Possibly. I couldn't say as I haven't looked at almost every software project in the world. I'm still wondering, why do they do it?
The information in the FILE field is already displayed in your editor.
The information the FILE field is generally used when a file has been moved from its original location (ie: in forked projects) so that the original may be quickly found. It is invaluable.
[CSH] ...if it were kept up to date, which we seem to have a great number of problems doing. Why is it invaluable to know the location within the repository when it was forked? It may have changed since then.
We should not misrepresent or not represent copyrights, so that should be there. We also reuse code from external projects and it's important to keep the copyright notices around. The less information there is to maintain, the more likely it is kept up to date.
I agree, but stuff like FILE, USAGE and PROGRAMMER don't require that much maintenance.
[CSH] Still a lot more than none. There are ~5700 C/C++ files in the reactos module. The average space taken up by the FILE, PURPOSE and PROGRAMMERS fields the 3 headers below is 183 bytes (without the revision changes descriptions). That is 994KB of text for 5700 files or an additional 1 minute download time (128Kbit ADSL). And let me just point out that ReactOS is still very, very small compared to other mainstream Operating Systems. Only a 16MB download at present. Normally I don't care much if additional space is used, but this is for information which is already present elsewhere.
Let's see some examples.
/* * COPYRIGHT: See COPYING in the top level directory * PROJECT: ReactOS kernel * FILE: ntoskrnl/ke/main.c * PURPOSE: Initalizes the kernel * * PROGRAMMERS: Alex Ionescu (cleaned up code, moved Executiv stuff to ex/init.c) * David Welch (welch@cwcom.net) */
So main.c initializes the kernel? Why don't we just rename that file to initialize_kernel.c? Or just initialize.c and rename the parent directory ke to kernel. Both would give us the same information.
/* * COPYRIGHT: See COPYING in the top level directory * PURPOSE: ReactOS kernel * FILE: ntoskrnl/ke/kqueue.c * PURPOSE: Implement device queues * * PROGRAMMERS: Alex Ionescu (alex@relsoft.net) - Several optimizations and implement * usage of Inserted flag + reformat and * add debug output. * David Welch (welch@mcmail.com) */
Device_queue.c anyone?
/* $Id: sem.c 15164 2005-05-09 01:38:29Z sedwards $ * * COPYRIGHT: See COPYING in the top level directory * PROJECT: ReactOS kernel * FILE: ntoskrnl/ke/sem.c * PURPOSE: Implements kernel semaphores * * PROGRAMMERS: David Welch (welch@mcmail.com) */
Semaphore.c? But wait, Subversion says Steven changed the file last time, yet he's not mentioned in the PROGRAMMERS field. Is Subversion lying? Was David the only person making changes to this file?
Actually, Subversion tells us that David didn't even write half of those lines in the newest revision. A total of 9 people contributed to the file in the newest revision: http://www.csh-consult.dk/sem_c.txt. What good is this header information to anyone?
Casper