[ros-dev] top-level source header

Wed Oct 19 19:31:18 CEST 2005

> -----Original Message-----
> From: ros-dev-bounces at reactos.org [mailto:ros-dev-bounces at 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 at 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 at relsoft.net) - Several optimizations and implement
 *                                                    usage of Inserted flag + reformat and
 *                                                    add debug output.
 *                  David Welch (welch at 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 at 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