[PATCH 1 of 6] histedit: move autoverb logic from torule to ruleeditor

[Augie Fackler]

On Wed, Jun 29, 2016 at 08:08:59PM -0700, Sean Farley wrote:
> Pierre-Yves David <pierre-yves.david at ens-lyon.org> writes:
>
> > On 06/22/2016 01:32 AM, Sean Farley wrote:
> >> # HG changeset patch
> >> # User Sean Farley <sean at farley.io>
> >> # Date 1464306370 25200
> >> #      Thu May 26 16:46:10 2016 -0700
> >> # Node ID 01212d1f93a2a6c5c736dc0c7d45ffd1930e5c2f
> >> # Parent  aa1d56003872cba207d908706da059141dd901a5
> >> # EXP-Topic autoverb
> >> histedit: move autoverb logic from torule to ruleeditor
> >
> > This series is taking me more time to review than it could. It mostly
> > come from the fact that the commit description are not very helpful to me.
> >
> > The descriptions in this series are extremely factual and descriptive
> > about the patch and does not contains much context. In practice, this
> > mean than the description mostly duplicate the patch information without
> > helping me to understand why the change is the right thing to do.
> >
> > For example, on this first changeset, you are moving logic X from class
> > A to class B. It would help me to know what is the semantic of class A
> > and B and why X fit better in B. Otherwise reviewing this series
> > requires me to re-analyze the whole source of histedit in order to
> > understand what is going on here. If you could write down the principle
> > you used decide how to move forward, it would allow me (and any other
> > reader) to first quickly validate the principle and then validate the
> > change based on that principle. Because of this, that series have been
> > delayed in my "second pass in more depth bucket for a couple of day"
> > (because I did not got time to make such second pass yet).
> >
> > Consider sending a V2 with more explanation if you want to speedup the
> > handling of that series.
>
> Sure, makes sense.

I agree that the log messages here could probably be better, but the
proposed functionality looks great to me, and the implementation seems
fine (so when a V2 lands, it should be a breeze).

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel