Hi, On Wed, 6 Nov 2019, Junio C Hamano wrote: > Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: > > > Your commit message title should be of the form "<component>: <change>", > > e.g.: > > > > rev-list: support --first-parent with --bisect* > > Good suggestion. > > > I would be much more laconic (in particular, omitting subjective terms > > like "minutiae" and "mountains of irrelevant data"), but perhaps that is > > just a matter of subjective style. > > FWIW, I had the same reaction. That part of the message was too > noisy without adding much actual value. > > >> Note, bisecting on --first-parent becomes part of findall's previously > >> existing pass-through as an "option state" flag. > > > > I don't understand this part. > > Me neither. > > > Also, clarify in the commit message somewhere that this commit does not > > change the behavior of "git bisect". > > s/\.$/ when used without the "--first-parent" option&/; you mean? > > > As for the diff, besides my comments below, a change in the user-facing > > documentation of "rev-list" is needed, since --bisect and --first-parent > > now work together. > > True. I too am, like you are, happy to see that these two options > made to work well together. > > Thanks, both, for the patch and useful comments. My own review on > it may take a bit more time. While I sadly won't have time to review this patch, let me first state that I am very excited that you revived this stalled patch. Thank you! In addition to Jonathan's comments, I would like to add another one: I would have loved for this patch to be split in two, the first one introducing the `bisect_flags` and using it with `BISECT_FIND_ALL` instead of doing the `find_all` thing, the second one building the first-parent feature on top. Thanks, Dscho
