Re: [PATCH 02/22] sequencer: use memoized sequencer directory path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kuba,

On Mon, 29 Aug 2016, Jakub Narębski wrote:

> W dniu 29.08.2016 o 10:04, Johannes Schindelin pisze:
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> >  builtin/commit.c |  2 +-
> >  sequencer.c      | 11 ++++++-----
> >  sequencer.h      |  5 +----
> >  3 files changed, 8 insertions(+), 10 deletions(-)
> 
> Just a sidenote: it would be probably easier to read with *.h before
> *.c (at least this particular one).

I agree, but I did not find any way to reorder this without substantial
manual work...

> > diff --git a/builtin/commit.c b/builtin/commit.c
> > index 77e3dc8..0221190 100644
> > --- a/builtin/commit.c
> > +++ b/builtin/commit.c
> > @@ -173,7 +173,7 @@ static void determine_whence(struct wt_status *s)
> >  		whence = FROM_MERGE;
> >  	else if (file_exists(git_path_cherry_pick_head())) {
> >  		whence = FROM_CHERRY_PICK;
> > -		if (file_exists(git_path(SEQ_DIR)))
> > +		if (file_exists(git_path_seq_dir()))
> >  			sequencer_in_use = 1;
> >  	}
> >  	else
> 
> So it is more "Use memoized sequencer directory path" rather than
> "sequencer: use memoized sequencer directory path" - it replaces
> all occurrences of SEQ_DIR,... that's why it can be removed from
> 'sequencer.h'.
> 
> Though perhaps I misunderstood "sequencer: " prefix there.  Don't
> mind me then.

The idea is that this path is declared and defined in the sequencer. There
are other call sites, too, so they have to be changed at the same time...

I'd really like to keep the "sequencer:" prefix because it is semantically
correct: this change is about the sequencer, not about the other call
sites.

Ciao,
Johannes

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]