[16/16] fsmonitor-settings: simplify initialization of settings data

Commit Message

Jeff Hostetler March 11, 2022, 9:15 p.m. UTC

From: Jeff Hostetler <[e-mail]>

fixup! fsmonitor: config settings are repository-specific

Signed-off-by: Jeff Hostetler <[e-mail]>
---
 fsmonitor-settings.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Junio C Hamano March 14, 2022, 7:49 p.m. UTC | #1

"Jeff Hostetler via GitGitGadget" <[e-mail]> writes:

> From: Jeff Hostetler <[e-mail]>
>
> fixup! fsmonitor: config settings are repository-specific

I cannot tell the validity of this change alone without the base
commit in part2 to choose between setting s->mode directly or
calling __set_disabled(r).

I've read all the 16 patches here, and I found that the majority of
them (except for "it is hard to tell why we are changing in 2.5; it
needs a better justification" ones like this) are "oops, the part2
patches had these obvious and trivial mistakes that we should have
fixed before we merged them to 'next', or even sending them to the
list in the first place" fixes.

Let's kick part2 out of 'next', and replace it with a corrected set
of patches and have people review them, this time for real, before
merging it back to 'next'.  I just do not get the good feel that the
series was adequately reviewed---if we have this many "oops" fixups
needed after getting merged to 'next'.

Thanks.



> Signed-off-by: Jeff Hostetler <[e-mail]>
> ---
>  fsmonitor-settings.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c
> index 3b54e7a51f6..757d230d538 100644
> --- a/fsmonitor-settings.c
> +++ b/fsmonitor-settings.c
> @@ -22,11 +22,10 @@ static void lookup_fsmonitor_settings(struct repository *r)
>  		return;
>  
>  	CALLOC_ARRAY(s, 1);
> +	s->mode = FSMONITOR_MODE_DISABLED;
>  
>  	r->settings.fsmonitor = s;
>  
> -	fsm_settings__set_disabled(r);
> -
>  	/*
>  	 * Overload the existing "core.fsmonitor" config setting (which
>  	 * has historically been either unset or a hook pathname) to

Jeff Hostetler March 15, 2022, 6:26 p.m. UTC | #2

On 3/14/22 3:49 PM, Junio C Hamano wrote:
> "Jeff Hostetler via GitGitGadget" <[e-mail]> writes:
> 
>> From: Jeff Hostetler <[e-mail]>
>>
>> fixup! fsmonitor: config settings are repository-specific
> 
> 
> Let's kick part2 out of 'next', and replace it with a corrected set
> of patches and have people review them, this time for real, before
> merging it back to 'next'.  I just do not get the good feel that the
> series was adequately reviewed---if we have this many "oops" fixups
> needed after getting merged to 'next'.

Yeah if you haven't merged part2 to master yet, wait and i'll
send a V7 of part2 that squashes in these fixups.

and addresses any other comments on part2.5 itself.

Thanks and sorry.
Jeff

Junio C Hamano March 15, 2022, 7:06 p.m. UTC | #3

Jeff Hostetler <[e-mail]> writes:

> Yeah if you haven't merged part2 to master yet, wait and i'll
> send a V7 of part2 that squashes in these fixups.
>
> and addresses any other comments on part2.5 itself.

Thanks.  I think that would be cleaner in the end.

Patch

diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c
index 3b54e7a51f6..757d230d538 100644
--- a/fsmonitor-settings.c
+++ b/fsmonitor-settings.c
@@ -22,11 +22,10 @@  static void lookup_fsmonitor_settings(struct repository *r)
 		return;
 
 	CALLOC_ARRAY(s, 1);
+	s->mode = FSMONITOR_MODE_DISABLED;
 
 	r->settings.fsmonitor = s;
 
-	fsm_settings__set_disabled(r);
-
 	/*
 	 * Overload the existing "core.fsmonitor" config setting (which
 	 * has historically been either unset or a hook pathname) to