From: Phillip Wood Date: Tue, 20 Aug 2019 18:37:16 GMT Subject: Re: [PATCH v3 0/6] rebase -i: support more options Message-ID: In-Reply-To: On 20/08/2019 18:53, Junio C Hamano wrote: > Phillip Wood writes: > >> Hi Rohit >> >> On 20/08/2019 04:45, Rohit Ashiwal wrote: >>> I've tries to incorporated all the suggestions. >> >> It is helpful if you can list the changes to remind us all what we >> said. (as a patch author I find composing that is helpful to remind me >> if there's anything I've forgotten to address) >> >> Also there are a couple of things that were discussed such as >> splitting up the author and passing it round as a > tz> tuple and testing a non-default timezone which aren't included - >> that's fine but it helps if you take a moment to explain why in the >> cover letter. >> >>> >>> Some points: >>> - According to v2.0.0's git-am.sh, ignore-date should override >>> committer-date-is-author-date. Ergo, we are not barfing out >>> when both flags are provided. >>> - Should the 'const' qualifier be removed[2]? Since it is leaving >>> a false impression that author should not be free()'d. >> >> The author returned by read_author_ident() is owned by the strbuf that >> you pass to read_author_ident() which is confusing. >> >> Best Wishes >> >> Phillip > > I've looked at this round, but will push out v2 for today's > integration cycle, mostly due to lack of time, but there do not seem > to be great difference between the iterations. > > The "ignore-date" step conflicts semantically with b0a31861 > ("sequencer: simplify root commit creation", 2019-08-19) but in a > good way. Without the clean-up b0a31861 makes, we need to munge the > timestamp in two places, but with it, there is only one place that > needs to modify the timestamp for the feature (in try_to_commit()). That was my hope > You may want to see if these "more options" topic can take advantage > of the "simplify root commit creation" by building on top of some > form of it (I do not know offhand if b0a31861 ("sequencer: simplify > root commit creation", 2019-08-19) must build on other two patches > to fix "rebase -i" or it can be split out as an independent > clean-up), and if it is feasible, work with your student to make it > happen, perhaps? If it is separated out from the first two there will be a minor textual conflict as the first patch in that series changes the root commit creation to stop it writing CHERRY_PICK_HEAD and then b0a31861 deletes the modified version but it shouldn't be a problem to resolve manually. Rohit do you want to try cherry-picking b0a31861 onto master and then rebasing your patches on top of it? - it would be nice to simplify things. Best Wishes Phillip > Thanks. >