----- Original Message -----
From: "Jerry James" <james(a)xemacs.org>
To: "Ben Wing" <ben(a)xemacs.org>
Cc: <xemacs-beta(a)xemacs.org>
Sent: Friday, February 14, 2003 9:09 AM
Subject: Re: [COMMIT] speedups to build process
Moving to XEmacs Beta. This is not a comment on this patch in
particular.
"Ben Wing" <ben(a)xemacs.org> wrote:
[snip]
> cvs server: Diffing lisp
> Index: lisp/autoload.el
[snip]
> - (generate-file-autoloads-1 file funlist))
> + (generate-autoload-ish-1
> + file
> + (replace-in-string (file-name-nondirectory file) "\\.elc?$"
"")
> + nil #'generate-file-autoloads-1
> + funlist))
> ;; #### jj, are C++ modules possible?
I don't rant very often, so I'm going to take the opportunity to let one
loose.:-) This rant is about the last line above, which is *not* part
of Ben's patch, so I'm not ranting in his direction. Here is a comment,
apparently directed to me, stuck in the middle of this code. I've only
been a reviewer for a very short time, and this was apparently put in
prior to that time. I wasn't subscribed to xemacs-patches then, so I
didn't see it. Why wasn't it sent to me in email, for heaven's sake?
(The answer to the question posed above, by the way, is a definite
maybe. We certainly can't deal with mangled C++ names, but wrapping the
symbols the module code needs to see in extern "C" might be sufficient.
However, I don't know what other beasties might raise their ugly heads.
Somebody will just have to try it and see what happens.)
Along the same lines I recently stumbled, quite by accident, across a
ChangeLog entry that says, in part:
* make-docfile.c:
* make-docfile.c (read_c_string):
* make-docfile.c (scan_c_file):
Fix shadowing warnings. NOTE: This was already fixed
awhile ago, but reverted by Jerry. Please be careful.
Same rant: why was I not sent email about this? There were no comments
in the code explaining that the "buf" -> "buff" name change was
to
prevent a shadowing warning. There were no remarks in the ChangeLog
that said anything like this either. There are no shadowing warnings on
any of my Unix test platforms, so I can only guess that the warnings
occur on Some Other Platform, with developers so braindead that they
think defining the symbol "buf" in system headers is a good idea. I
sent the patch that made the name change with a ChangeLog entry that
explicitly stated that I was making this particular change to reduce the
size of the diff against the FSF version, and it was approved.
Just how, exactly, was I supposed to "be careful"? I agree that
somebody was not careful, but it was not me. It was the person who
originally fixed the warning and didn't leave any clues behind to let
anybody else know what they had done. But I'm the one being fingered as
careless in a file that is now being circulated around the globe. I
would have pointed all this out if somebody had bothered to send me
email first! So the point of this much-too-long rant is to send email
when you have something to say to another developer. Inserting comments
into code and ChangeLogs is not effective communication.
Okay, I'm done ranting now. Time for a nice relaxing dip in the ocean.
Oh wait, I'm in Kansas, darn it....
ok, i wrote this "be careful" message. in this case, the problem is not
"Some
Other Platform" but the fact that there was both a global `buf' and a `buf'
parameter to write_c_args. Anyone compiling with gcc -Wshadow will see this, on
all platforms. this is supposed to be the default but people changing --cflags
would override it and usually forgot to stick it back in, so i fixed this
recently and everyone should get -Wshadow now.
i agree i should have put in a comment explaining this. however, in general
when synching, assume that any change made by XEmacs has a purpose and don't
eliminate it unless you understand why the change was made and you're sure it's
no longer necessary.
--
Jerry James, native Californian, currently reaching for the Prozac
http://www.ittc.ku.edu/~james/