Change in compile.el

Change in compile.el

[Stefan Monnier]

About the following change:

2004-02-16  Alfred M. Szmidt  <ams@kemisten.nu>  (tiny change)

	* progmodes/compile.el (compilation-directory): New defvar.
	(compile): Save current directory in compilation-directory.
	(recompile): Bind default-directory to compilation-directory if
	that is non-nil.

Has somebody tested this change?  It doesn't work for me because it sets
a global variable and thus forces the directory to be the same as "the last
compile", rather than the same as "this compile".

I.e. it breaks in those cases where the old code did the right thing
(i.e. when you go to a compile-mode buffer and do `recompile' there).


        Stefan


PS: Also, would it be possible to try and refrain from changing compile.el
until the newer version is installed?

Re: Change in compile.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: 16 Feb 2004 15:08:48 -0500
> 
> 2004-02-16  Alfred M. Szmidt  <ams@kemisten.nu>  (tiny change)
> 
> 	* progmodes/compile.el (compilation-directory): New defvar.
> 	(compile): Save current directory in compilation-directory.
> 	(recompile): Bind default-directory to compilation-directory if
> 	that is non-nil.
> 
> Has somebody tested this change?

Richard asked me to commit this.  I didn't test it, just proofread it.

> I.e. it breaks in those cases where the old code did the right thing
> (i.e. when you go to a compile-mode buffer and do `recompile' there).

Feel free to fix the code so that this special case isn't broken, or
even take out this change entirely if you think it does more harm than
good.

> PS: Also, would it be possible to try and refrain from changing compile.el
> until the newer version is installed?

How is one supposed to know that a new version is in the works?  We
don't really have a machinery for approving patches, so there's no
good way to coordinate patches.

Coordinating patches [was Re: Change in compile.el]

[Kim F. Storm]

Eli Zaretskii <eliz@elta.co.il> writes:

> How is one supposed to know that a new version is in the works?  We
> don't really have a machinery for approving patches, so there's no
> good way to coordinate patches.

We have been discussing the new compile.el on the mailing lists.

Also, the patch you applied to keyboard.c also had a nasty bug which
I had already raised on the mailing list, and thus explained why it
should not be applied in its current form.  I was quite surprised
to see it installed anyway.

Given several incidents over the last few weeks where patches do more
harm than good, I think we need to tighten the procedure of applying
"3rd party" patches.  

What about this simple procedure:

Before applying a 3rd party patch, a message is sent to emacs-devel
with the subject:

        PATCH REVIEW: [title from original mail]

If no objections are received to that mail in 36 hours, you are
free to go ahead an install it.  

If it later turns out that there were problems with the patch anyway,
we are all to blame :-)

I know it is more work, but so is discussing and fixing the problems 
of a bad patch afterwards...


BTW we shall NOT follow this procudure for changes made by the project
members -- that would be very counter-productive.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Coordinating patches [was Re: Change in compile.el]

[Eli Zaretskii]

> From: storm@cua.dk (Kim F. Storm)
> Date: 17 Feb 2004 11:16:11 +0100
> 
> We have been discussing the new compile.el on the mailing lists.

We are discussing a lot of things, many of them don't end in a patch
and not necessarily are meant to.  Even if code fragments are being
posted and discussed, there's no way to know that someone is actively
working on a change.

> Also, the patch you applied to keyboard.c also had a nasty bug which
> I had already raised on the mailing list, and thus explained why it
> should not be applied in its current form.  I was quite surprised
> to see it installed anyway.

Richard asked me to install it a long time ago, and I guess I've
missed your objections (and so did Richard, it seems, since he never
drew my attention to your objections).  As long as we rely on humans
to pay attention, these things can happen.

> Given several incidents over the last few weeks where patches do more
> harm than good, I think we need to tighten the procedure of applying
> "3rd party" patches.  

I don't have anything against formalizing patch approval (in fact, I
suggested long ago that we do that for _all_ non-trivial changes, but
was voted down).  However, when Richard or some other core maintainer
asks me to install a patch, I don't regard that as ``3rd party'', I'm
sure you understand why.

> Before applying a 3rd party patch, a message is sent to emacs-devel
> with the subject:
> 
>         PATCH REVIEW: [title from original mail]
> 
> If no objections are received to that mail in 36 hours, you are
> free to go ahead an install it.  

People who object to a patch can voice their objections as things are
today: simply assume that every patch suggested on one of the 3
mailing lists could be installed RSN.  In other words, if you care
about the quality of the CVS code, please take time to review patches
that ``3rd parties'' are posting on these 3 lists.  No need to wait
for an announcement that a certain patch is about to be installed.

Re: Coordinating patches [was Re: Change in compile.el]

[Richard Stallman]

    What about this simple procedure:

    Before applying a 3rd party patch, a message is sent to emacs-devel
    with the subject:

In an ideal world, this might be a good idea, but I doubt it is
feasible here and now.

I asked Eli to install the patches because he said he was willing to
do that.  However, his available time has been just barely enough time
to do that.  If I were to ask him to do this extra work for each
change, the most likely result is that he'd have to say no.

Re: Coordinating patches [was Re: Change in compile.el]

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Wed, 18 Feb 2004 03:38:32 -0500
> 
> I asked Eli to install the patches because he said he was willing to
> do that.  However, his available time has been just barely enough time
> to do that.  If I were to ask him to do this extra work for each
> change, the most likely result is that he'd have to say no.

Probably.

The more I think about what I wrote earlier in this thread, the more I
like it: if patches suggested by people other than the core
maintainers will get reviewed and responded to by the developers soon
after they are posted to the various lists, we will win on 2 counts:
(1) more potential bugs will be detected before they get into the CVS,
and (2) it will make life easier for Richard who up until now did that
almost single-handedly.

Re: Coordinating patches [was Re: Change in compile.el]

[Kim F. Storm]

Eli Zaretskii <eliz@elta.co.il> writes:

> > From: Richard Stallman <rms@gnu.org>
> > Date: Wed, 18 Feb 2004 03:38:32 -0500
> > 
> > I asked Eli to install the patches because he said he was willing to
> > do that.  However, his available time has been just barely enough time
> > to do that.  If I were to ask him to do this extra work for each
> > change, the most likely result is that he'd have to say no.
> 
> Probably.
> 
> The more I think about what I wrote earlier in this thread, the more I
> like it: if patches suggested by people other than the core
> maintainers will get reviewed and responded to by the developers soon
> after they are posted to the various lists, we will win on 2 counts:
> (1) more potential bugs will be detected before they get into the CVS,
> and (2) it will make life easier for Richard who up until now did that
> almost single-handedly.


Exactly -- if I see a new patch that I think it looks ok to include, I
can check for legal papers from poster and if ok, immediately post a
PATCH REVIEW message to emacs-devel, and in a couple of days I can
just install it if nobody has objected to do so.

No double work, no forgotten patches, no patches committed without
prior review... ?

Maybe we should say that the review period is 3 days, i.e. if no
comments in 3 days, it is ok to commit.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Coordinating patches [was Re: Change in compile.el]

[Eli Zaretskii]

> From: storm@cua.dk (Kim F. Storm)
> Date: 18 Feb 2004 13:52:04 +0100
> 
> Exactly -- if I see a new patch that I think it looks ok to include, I
> can check for legal papers from poster and if ok, immediately post a
> PATCH REVIEW message to emacs-devel, and in a couple of days I can
> just install it if nobody has objected to do so.

And if someone sees a patch that they think is not okay, publishing
the comments and objections would be also very useful.

> Maybe we should say that the review period is 3 days, i.e. if no
> comments in 3 days, it is ok to commit.

I'd give it a week, more during important public holidays.

Re: Coordinating patches [was Re: Change in compile.el]

[Richard Stallman]

    Exactly -- if I see a new patch that I think it looks ok to include, I
    can check for legal papers from poster and if ok, immediately post a
    PATCH REVIEW message to emacs-devel, and in a couple of days I can
    just install it if nobody has objected to do so.

I would appreciate it if you and others did this sometimes.

    Maybe we should say that the review period is 3 days, i.e. if no
    comments in 3 days, it is ok to commit.

Please make it 4 days, because sometimes it will take that long
for me to respond at all.