Casper Hornstrup wrote:
-----Original Message-----
From: ros-dev-bounces(a)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?
I gave an example why below... this question is irrelevant if you had
read my whole email. I'll say the sample again: documentators.
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.
You keep avoiding the point: these files might one day find themselves
OUTSIDE the repository!
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.
He'll need to go through 30-40 usernames, then figure out which username
actually wrote useful code. That would take HOURS.
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.
Funny how every other project can...but we can't? What's to hard about
adding a purpose to a file? If I can do it, so can other devs (and they
do, almost everyone here uses a top-level header).
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.
Nor do you need legs, arms, ears and eyes to live.
Maybe that code
could need some refactoring?
If you think refactoring solves the necessity of comments, I'm afraid
you need to revise that notion. Comments are not meant to explain WHAT
the code does all the time, but more importantly WHY it does it. When
you deal with technical code, sometimes you can do things which don't
make a lot of sense unless they are commented. If you look at the DDK
sample code, some race conditions were found by Microsoft developers
after YEARS of testing. And sometimes a line or two of code had to be
added. If you don't comment it, nobody is going to guess why it's there,
as many times as you refactor.
Not commenting code is totally bad practice.
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?
I've already answered this. Source code is not only for the "elite",
you
know. If only programmers would look at source code, then FOSS wouldn't
have gained the major popularity it has now. The whole point is that
even a 10 year old can look at source code and fix something really
stupid, like:
/* Check if we should do FOO */
if (Foo == FALSE) DoFoo;
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?
Look at WINE, Linux, Solaris, Windows...
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.
Better then nothing!
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.
Now that's ironic. When I argued about some build changes which caused a
~2-4MB reduction in that 16MB I was told that "Who cares about 2-4MB".
Now we have to care about 994KB?
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(a)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(a)relsoft.net) - Several optimizations and implement
* usage of Inserted flag + reformat
and
* add debug output.
* David Welch (welch(a)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(a)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?
NONE. That's why we need to get people to start using it properly (let's
have a vote). If peopel decide against top-level headers, then fine, I
won't insist.
Casper
Best regards,
Alex Ionescu