[commit] Add add_setshow_enum_cmd, use in mips

[commit] Add add_setshow_enum_cmd, use in mips

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 36 bytes --]

Another cleanup.

committed,
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 5148 bytes --]

2004-10-30  Andrew Cagney  <cagney@gnu.org>

	* cli/cli-decode.c (add_setshow_enum_cmd): New function.
	* command.h (add_setshow_enum_cmd): Declare.
	* mips-tdep.c (_initialize_mips_tdep): Use add_setshow_enum_cmd.

Index: command.h
===================================================================
RCS file: /cvs/src/src/gdb/command.h,v
retrieving revision 1.41
diff -p -u -r1.41 command.h
--- command.h	28 Jul 2004 19:42:00 -0000	1.41
+++ command.h	30 Oct 2004 17:09:25 -0000
@@ -223,6 +223,18 @@ extern struct cmd_list_element *add_set_
 						  const char **var,
 						  char *doc,
 						  struct cmd_list_element **list);
+extern void add_setshow_enum_cmd (char *name,
+				  enum command_class class,
+				  const char *enumlist[],
+				  const char **var,
+				  const char *set_doc,
+				  const char *show_doc,
+				  const char *help_doc,
+				  const char *print,
+				  cmd_sfunc_ftype *set_func,
+				  cmd_sfunc_ftype *show_func,
+				  struct cmd_list_element **set_list,
+				  struct cmd_list_element **show_list);
 
 extern void add_setshow_auto_boolean_cmd (char *name,
 					  enum command_class class,
Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.342
diff -p -u -r1.342 mips-tdep.c
--- mips-tdep.c	30 Oct 2004 16:53:22 -0000	1.342
+++ mips-tdep.c	30 Oct 2004 17:09:26 -0000
@@ -6029,31 +6029,31 @@ _initialize_mips_tdep (void)
 		  &showmipscmdlist, "show mips ", 0, &showlist);
 
   /* Allow the user to override the saved register size. */
-  deprecated_add_show_from_set (add_set_enum_cmd ("saved-gpreg-size",
-						  class_obscure,
-						  size_enums,
-						  &mips_abi_regsize_string, "\
-Set size of general purpose registers saved on the stack.\n\
+  add_setshow_enum_cmd ("saved-gpreg-size", class_obscure,
+			size_enums, &mips_abi_regsize_string, "\
+Set size of general purpose registers saved on the stack.\n", "\
+Show size of general purpose registers saved on the stack.\n", "\
 This option can be set to one of:\n\
   32    - Force GDB to treat saved GP registers as 32-bit\n\
   64    - Force GDB to treat saved GP registers as 64-bit\n\
   auto  - Allow GDB to use the target's default setting or autodetect the\n\
           saved GP register size from information contained in the executable.\n\
-          (default: auto)", &setmipscmdlist), &showmipscmdlist);
+          (default: auto)", "\
+Size of general purpose registers saved on the stack is %s.\n",
+			NULL, NULL, &setmipscmdlist, &showmipscmdlist);
 
   /* Allow the user to override the argument stack size. */
-  deprecated_add_show_from_set
-    (add_set_enum_cmd ("stack-arg-size",
-		       class_obscure,
-		       size_enums,
-		       &mips_stack_argsize_string, "\
-Set the amount of stack space reserved for each argument.\n\
+  add_setshow_enum_cmd ("stack-arg-size", class_obscure,
+		       size_enums, &mips_stack_argsize_string, "\
+Set the amount of stack space reserved for each argument.\n", "\
+Show the amount of stack space reserved for each argument.\n", "\
 This option can be set to one of:\n\
   32    - Force GDB to allocate 32-bit chunks per argument\n\
   64    - Force GDB to allocate 64-bit chunks per argument\n\
   auto  - Allow GDB to determine the correct setting from the current\n\
-          target and executable (default)", &setmipscmdlist),
-     &showmipscmdlist);
+          target and executable (default)", "\
+The amount of stack space reserved for each argument is %s.\n",
+			NULL, NULL, &setmipscmdlist, &showmipscmdlist);
 
   /* Allow the user to override the ABI. */
   c = add_set_enum_cmd
Index: cli/cli-decode.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-decode.c,v
retrieving revision 1.41
diff -p -u -r1.41 cli-decode.c
--- cli/cli-decode.c	30 Jul 2004 17:55:47 -0000	1.41
+++ cli/cli-decode.c	30 Oct 2004 17:09:26 -0000
@@ -402,6 +402,34 @@ add_set_enum_cmd (char *name,
   return c;
 }
 
+/* Add element named NAME to command list LIST (the list for set or
+   some sublist thereof).  CLASS is as in add_cmd.  ENUMLIST is a list
+   of strings which may follow NAME.  VAR is address of the variable
+   which will contain the matching string (from ENUMLIST).  */
+
+void
+add_setshow_enum_cmd (char *name,
+		      enum command_class class,
+		      const char *enumlist[],
+		      const char **var,
+		      const char *set_doc,
+		      const char *show_doc,
+		      const char *help_doc,
+		      const char *print,
+		      cmd_sfunc_ftype *set_func,
+		      cmd_sfunc_ftype *show_func,
+		      struct cmd_list_element **set_list,
+		      struct cmd_list_element **show_list)
+{
+  struct cmd_list_element *c;
+  add_setshow_cmd_full (name, class, var_enum, var,
+			set_doc, show_doc, help_doc, print,
+			set_func, show_func,
+			set_list, show_list,
+			&c, NULL);
+  c->enums = enumlist;
+}
+
 /* Add an auto-boolean command named NAME to both the set and show
    command list lists.  CLASS is as in add_cmd.  VAR is address of the
    variable which will contain the value.  DOC is the documentation

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Sat, 30 Oct 2004 12:11:14 -0400
> From: Andrew Cagney <cagney@gnu.org>
> 
> Another cleanup.
> 
> committed,

FWIW, I don't see why is this a ``cleanup'', nor why should it be
committed without going through an RFA.  You are introducing a new
function, so at the very least the reasons for its introduction should
have been explained and other maintainers should have been given an
opportunity to suggest alternative designs.

Sorry if this has been discussed in the past (but if so, a URL
pointing to those discussions should have been mentioned).

In any case, please add the documentation of this function to
gdbint.texinfo.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Andrew Cagney]

Eli Zaretskii wrote:
>>Date: Sat, 30 Oct 2004 12:11:14 -0400
>>From: Andrew Cagney <cagney@gnu.org>
>>
>>Another cleanup.
>>
>>committed,
> 
> 
> FWIW, I don't see why is this a ``cleanup'', nor why should it be
> committed without going through an RFA.  You are introducing a new
> function, so at the very least the reasons for its introduction should
> have been explained and other maintainers should have been given an
> opportunity to suggest alternative designs.

The missing add_setshow_enum_cmd both goes with, and is consistent, with 
the existing add_setshow zinteger, boolean, auto_boolean, filename, 
string, uinteger command functions  I just plugged an obvious hole, and 
in the process fixed an i18n problem.

Andrew

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Sun, 31 Oct 2004 18:01:30 -0500
> From: Andrew Cagney <cagney@gnu.org>
> Cc: gdb-patches@sources.redhat.com
> 
> The missing add_setshow_enum_cmd both goes with, and is consistent, with 
> the existing add_setshow zinteger, boolean, auto_boolean, filename, 
> string, uinteger command functions

I didn't say the patch was unreasonable, just that it should have been
an RFA, since it is by no means ``obvious''.

What about documenting the function?

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Daniel Jacobowitz]

On Mon, Nov 01, 2004 at 06:41:12AM +0200, Eli Zaretskii wrote:
> > Date: Sun, 31 Oct 2004 18:01:30 -0500
> > From: Andrew Cagney <cagney@gnu.org>
> > Cc: gdb-patches@sources.redhat.com
> > 
> > The missing add_setshow_enum_cmd both goes with, and is consistent, with 
> > the existing add_setshow zinteger, boolean, auto_boolean, filename, 
> > string, uinteger command functions
> 
> I didn't say the patch was unreasonable, just that it should have been
> an RFA, since it is by no means ``obvious''.

Why?  There is no maintainer for this area, and Andrew is entitled to
approve patches himself, so an RFA (request for approval) seems
unnecessary.

-- 
Daniel Jacobowitz

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Mon, 1 Nov 2004 00:12:57 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Andrew Cagney <cagney@gnu.org>, gdb-patches@sources.redhat.com
> 
> There is no maintainer for this area, and Andrew is entitled to
> approve patches himself, so an RFA (request for approval) seems
> unnecessary.

Where do you see such a rule?  It's not in MAINTAINERS, AFAICT.

The rule that we do have is that if a certain maintenance area has no
responsible maintainer, the _responsibility_ falls to the head
maintainer.  But my interpretation of this is that the responsibility
is for reviewing patches, not for applying own patches without asking
for approval.  That's because it doesn't make sense to me to decide
that whenever some area maintainer steps down, the head maintainer is
automatically promoted to be an expert in that area.  If you were not
an expert in some area, the fact that the expert disappeared doesn't
make you an expert, just the one who is burdened with more duties.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Daniel Jacobowitz]

On Mon, Nov 01, 2004 at 11:09:51PM +0200, Eli Zaretskii wrote:
> > Date: Mon, 1 Nov 2004 00:12:57 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: Andrew Cagney <cagney@gnu.org>, gdb-patches@sources.redhat.com
> > 
> > There is no maintainer for this area, and Andrew is entitled to
> > approve patches himself, so an RFA (request for approval) seems
> > unnecessary.
> 
> Where do you see such a rule?  It's not in MAINTAINERS, AFAICT.
> 
> The rule that we do have is that if a certain maintenance area has no
> responsible maintainer, the _responsibility_ falls to the head
> maintainer.

No, we don't have any rule like that.  If a certain area has no
maintainers, the responsibility falls to the global maintainers - all
of us.

>  But my interpretation of this is that the responsibility
> is for reviewing patches, not for applying own patches without asking
> for approval.  That's because it doesn't make sense to me to decide
> that whenever some area maintainer steps down, the head maintainer is
> automatically promoted to be an expert in that area.  If you were not
> an expert in some area, the fact that the expert disappeared doesn't
> make you an expert, just the one who is burdened with more duties.

If there were an expert, who cared enough to disagree with any patches
applied, perhaps that person should be maintaining said area.  I only
see a point for maintainers to post RFAs when (A) they can not approve
the patch themselves or (B) they are not confident/happy/sure with the
approach.  We don't operate on consensus (obviously enough from reading
the lists).

-- 
Daniel Jacobowitz

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Mon, 1 Nov 2004 17:37:16 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: cagney@gnu.org, gdb-patches@sources.redhat.com
> 
> I only see a point for maintainers to post RFAs when (A) they can
> not approve the patch themselves or (B) they are not
> confident/happy/sure with the approach.

I'm astonished: you are, in effect, saying that the patch review
process exists only because some meaningless bureaucratic rule does
not permit a single person to do whatever he/she wants.  I kinda
thought that the patch review is the default, except when the patch
comes from an expert whom we trust to be good enough not to need that.

> We don't operate on consensus

I thought we should.  If not, I don't see much sense in the machinery
that we have in place.  To me, the reason for our procedures is to
produce quality code, not just to make an impression of due process.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Daniel Jacobowitz]

[Eli, I apologize for the delay in responding; I didn't mean to drop
this conversation on the floor, but I haven't had much time for GDB
lately.]

On Tue, Nov 02, 2004 at 06:44:48AM +0200, Eli Zaretskii wrote:
> > Date: Mon, 1 Nov 2004 17:37:16 -0500
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: cagney@gnu.org, gdb-patches@sources.redhat.com
> > 
> > I only see a point for maintainers to post RFAs when (A) they can
> > not approve the patch themselves or (B) they are not
> > confident/happy/sure with the approach.
> 
> I'm astonished: you are, in effect, saying that the patch review
> process exists only because some meaningless bureaucratic rule does
> not permit a single person to do whatever he/she wants.  I kinda
> thought that the patch review is the default, except when the patch
> comes from an expert whom we trust to be good enough not to need that.

I'm certainly not trying to say that!  But self-review seems like a
reasonable practice to me and my experience reading gdb-patches shows
it to be a pretty common one.  Most patches are self-approved.  Some
maintainers tend to post patches to areas without specific maintainers
as RFA's; others don't.

The other thing experience tells me is that patches posted as an RFA,
by someone who could self-approve it, only very rarely get reviewed.
Often they sit for ages.

> > We don't operate on consensus
> 
> I thought we should.  If not, I don't see much sense in the machinery
> that we have in place.  To me, the reason for our procedures is to
> produce quality code, not just to make an impression of due process.

Again, I feel disconnected.  I agree with your premises but not your
conclusions.  If there is no expert in the area, who will know more
about the code than the global maintainers generally, why ask for
review?  I assume that all of the active maintainers can handle coding
style, and general "is this a gross hack" checks, on their own.

-- 
Daniel Jacobowitz

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Mon, 8 Nov 2004 20:14:58 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: cagney@gnu.org, gdb-patches@sources.redhat.com
> 
> But self-review seems like a
> reasonable practice to me and my experience reading gdb-patches shows
> it to be a pretty common one.  Most patches are self-approved.  Some
> maintainers tend to post patches to areas without specific maintainers
> as RFA's; others don't.

I think self-approved patches are only appropriate when the person who
submits them is an expert in the area of the patch.

> The other thing experience tells me is that patches posted as an RFA,
> by someone who could self-approve it, only very rarely get reviewed.
> Often they sit for ages.

We could have some rule that, when there's no specific maintainer for
an area, a patch posted as an RFA could go in if unreviewed for some
time.

> If there is no expert in the area, who will know more about the code
> than the global maintainers generally, why ask for review?

The fact that there's no appointed area maintainer does not mean that
there are no people who know about the related code.  It could just
mean that no maintainer feels he/she can always be responsive enough
to step forward and suggest themselves as area maintainers for that
area.

> I assume that all of the active maintainers can handle coding
> style, and general "is this a gross hack" checks, on their own.

Matters of design are always best handled by peer reviews, in my
experience.  Something that isn't a gross hack can still be not the
best design.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Andrew Cagney]

This whole thread is stupid.

Before committing a patch as I did, I ask the question:

``Could a review significantly alter the change?''

Conversely, when reviewing a change, I ask:

``Is the objection going to significantly alter the change?''

Here, I cloned a pre-existing interface, adding another variant.  Anyone 
on this list with the problem I hit would have come up with an identical 
change.

Waiting a week would have achieved what?

Andrew

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Daniel Jacobowitz]

On Tue, Nov 09, 2004 at 10:27:52AM -0500, Andrew Cagney wrote:
> This whole thread is stupid.

Thank you for being respectful to your fellow developers.  It's nice to
know we're appreciated.

-- 
Daniel Jacobowitz

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Tue, 9 Nov 2004 13:42:21 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sources.redhat.com
> 
> On Tue, Nov 09, 2004 at 10:27:52AM -0500, Andrew Cagney wrote:
> > This whole thread is stupid.
> 
> Thank you for being respectful to your fellow developers.  It's nice to
> know we're appreciated.

Same here.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Wed, 10 Nov 2004 06:26:48 +0200
> From: "Eli Zaretskii" <eliz@gnu.org>
> CC: cagney@gnu.org, gdb-patches@sources.redhat.com
> 
> > On Tue, Nov 09, 2004 at 10:27:52AM -0500, Andrew Cagney wrote:
> > > This whole thread is stupid.
> > 
> > Thank you for being respectful to your fellow developers.  It's nice to
> > know we're appreciated.
> 
> Same here.

[Having given myself a few hours to cool down, I'm breathing again, and
so can now try to respond rationally rather than emotionally.]

> Before committing a patch as I did, I ask the question:
> 
> ``Could a review significantly alter the change?''
> 
> Conversely, when reviewing a change, I ask:
> 
> ``Is the objection going to significantly alter the change?''
> 
> Here, I cloned a pre-existing interface, adding another variant.  Anyone 
> on this list with the problem I hit would have come up with an identical 
> change.
> 
> Waiting a week would have achieved what?

Let me turn the table, Andrew, and ask you what possible harm could be
caused by posting the patch for review?  At best, no one would have
responded and you'd be committing the patch a week later.  No harm
done; case closed.  At worst, someone _would_ have responded, and the
patch would have been committed 7 days, instead of a week, later,
after some short discussion.  Again, no harm done.

So there're no real disadvantages to posting the patch as an RFA, and
a week doesn't seem like a high price to pay for having everybody
happy.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Mark Kettenis]

   Date: Wed, 10 Nov 2004 22:48:40 +0200
   From: "Eli Zaretskii" <eliz@gnu.org>

   > Before committing a patch as I did, I ask the question:
   > 
   > ``Could a review significantly alter the change?''
   > 
   > Conversely, when reviewing a change, I ask:
   > 
   > ``Is the objection going to significantly alter the change?''
   > 
   > Here, I cloned a pre-existing interface, adding another variant.  Anyone 
   > on this list with the problem I hit would have come up with an identical 
   > change.
   > 
   > Waiting a week would have achieved what?

   Let me turn the table, Andrew, and ask you what possible harm could be
   caused by posting the patch for review?  At best, no one would have
   responded and you'd be committing the patch a week later.  No harm
   done; case closed.  At worst, someone _would_ have responded, and the
   patch would have been committed 7 days, instead of a week, later,
   after some short discussion.  Again, no harm done.

Sorry, but I have to disagree here.  For the (unfortunately) limited
number of people that contribute several patches in a week this is a
significant problem.  When I'm working on a particular area I often
find myself making multiple changes to the same file.  If I have to
post a patch and wait a week before I can check it in, I have two
options:

1. Juggle with the patches for a week, risking an accidental commit of
   stuff belonging to a different patch to the same file, or dropping
   a patch completely in the process.

2. Postpone further work on that part of GDB until the week is over
   and the patch has been committed.

Neither option is good for GDB.

Mark

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Wed, 10 Nov 2004 22:42:14 +0100 (CET)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: cagney@gnu.org, drow@false.org, gdb-patches@sources.redhat.com
> 
> Sorry, but I have to disagree here.  For the (unfortunately) limited
> number of people that contribute several patches in a week this is a
> significant problem.  When I'm working on a particular area I often
> find myself making multiple changes to the same file.  If I have to
> post a patch and wait a week before I can check it in, I have two
> options:
> 
> 1. Juggle with the patches for a week, risking an accidental commit of
>    stuff belonging to a different patch to the same file, or dropping
>    a patch completely in the process.
> 
> 2. Postpone further work on that part of GDB until the week is over
>    and the patch has been committed.
> 
> Neither option is good for GDB.

That might be tough, but we all do precisely that when the file in
question is not in our maintainership area.

Mind you, I'm not suggesting that you should post an RFA in x86 files,
as you are the area maintainer for those.  I'm talking about files for
which there's no area maintainer.  The idea being that we are all
collectively responsible for such files, so the patch should be
approved collectively rather than unilaterally.

Yes, it slows down the development a bit, but I don't think the patch
rate is our most important goal.  The rate is important, but code
quality and clean design are IMHO more important.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Daniel Jacobowitz]

On Thu, Nov 11, 2004 at 01:25:05AM +0200, Eli Zaretskii wrote:
> > Date: Wed, 10 Nov 2004 22:42:14 +0100 (CET)
> > From: Mark Kettenis <mark.kettenis@xs4all.nl>
> > CC: cagney@gnu.org, drow@false.org, gdb-patches@sources.redhat.com
> > 
> > Sorry, but I have to disagree here.  For the (unfortunately) limited
> > number of people that contribute several patches in a week this is a
> > significant problem.  When I'm working on a particular area I often
> > find myself making multiple changes to the same file.  If I have to
> > post a patch and wait a week before I can check it in, I have two
> > options:
> > 
> > 1. Juggle with the patches for a week, risking an accidental commit of
> >    stuff belonging to a different patch to the same file, or dropping
> >    a patch completely in the process.
> > 
> > 2. Postpone further work on that part of GDB until the week is over
> >    and the patch has been committed.
> > 
> > Neither option is good for GDB.

I completely agree with Mark.

> That might be tough, but we all do precisely that when the file in
> question is not in our maintainership area.

You've said that quite a few times now :-)  But this isn't a documented
policy, and it doesn't seem to be a widely followed one, either.  This
seems to be the heart of our disagreement.

> Yes, it slows down the development a bit, but I don't think the patch
> rate is our most important goal.  The rate is important, but code
> quality and clean design are IMHO more important.

We can't improve the code quality and design, which have suffered over
time, if we can't make progress on patches.  Indeed, the longer it
takes for every single change to go in, the less incentive there is to
clean anything up!  I'm sure losing incentive to fix things.

-- 
Daniel Jacobowitz

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Eli Zaretskii]

> Date: Wed, 10 Nov 2004 18:41:44 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Mark Kettenis <mark.kettenis@xs4all.nl>, cagney@gnu.org,
> 	gdb-patches@sources.redhat.com
> 
> > That might be tough, but we all do precisely that when the file in
> > question is not in our maintainership area.
> 
> You've said that quite a few times now :-)  But this isn't a documented
> policy, and it doesn't seem to be a widely followed one, either.

What Andrew did isn't documented, either.

> We can't improve the code quality and design, which have suffered over
> time, if we can't make progress on patches.  Indeed, the longer it
> takes for every single change to go in, the less incentive there is to
> clean anything up!  I'm sure losing incentive to fix things.

Then let's abandon the patch reviewing procedure completely and just
commit whatever each one of us global maintainers finds appropriate.
There are projects that actually work that way (Emacs, for example).
I have no problem with that, as long as it is consistent and
documented.

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Daniel Jacobowitz]

On Thu, Nov 11, 2004 at 01:54:16AM +0200, Eli Zaretskii wrote:
> > We can't improve the code quality and design, which have suffered over
> > time, if we can't make progress on patches.  Indeed, the longer it
> > takes for every single change to go in, the less incentive there is to
> > clean anything up!  I'm sure losing incentive to fix things.
> 
> Then let's abandon the patch reviewing procedure completely and just
> commit whatever each one of us global maintainers finds appropriate.
> There are projects that actually work that way (Emacs, for example).
> I have no problem with that, as long as it is consistent and
> documented.

I've suggested something along those lines (not "abandoning patch
reviewing", but giving more authority to the global maintainers)
several times in the past, and met with mixed responses.  I hope to
eventually discuss this question with the GDB Steering Committee, and
get the result documented, one way or another.

-- 
Daniel Jacobowitz

Re: [commit] Add add_setshow_enum_cmd, use in mips

[Joel Brobecker]

> > Then let's abandon the patch reviewing procedure completely and just
> > commit whatever each one of us global maintainers finds appropriate.
> > There are projects that actually work that way (Emacs, for example).
> > I have no problem with that, as long as it is consistent and
> > documented.

FWIW, I am in favor of such an approach. At AdaCore, this is pretty much
how we work, and we have found that it works very well. The review
happens after commit. Some times we screw up, and we have to either
fix the screwup or revert, but most of the time the change is correct.
All in all, we save a lot of time this way. The part that we realize
is that with such a model, being questioned on a patch and maybe having
to revert it is considered normal. So we don't take any offense at
such comments, and don't consider this as a judgement of our abilities.

We also rely on the engineers' judgement to call for a review before
commit when this make sense. For instance, when a change is trickier
than usual, or he's unsure of his fix, or when something needs a design
that could benefit from peers' feedback, or when something impacts
a lot of code, etc.

But all in all, it is largely more effective for us to commit and
then review.

> I've suggested something along those lines (not "abandoning patch
> reviewing", but giving more authority to the global maintainers)
> several times in the past, and met with mixed responses.  I hope to
> eventually discuss this question with the GDB Steering Committee, and
> get the result documented, one way or another.


-- 
Joel