Subject: Re: [PATCH v8 10/11] sequencer.c: define describe_cleanup_mode

From: Denton Liu <[email-masked]>
Date: Mon, 18 Mar 2019 13:30:07 -0700

On Mon, Mar 18, 2019 at 04:04:22PM -0400, Eric Sunshine wrote:
> On Sun, Mar 17, 2019 at 6:16 AM Denton Liu <[email-masked]> wrote:
> > Define a function which allows us to get the string configuration value
> > of a enum commit_msg_cleanup_mode. This is done by refactoring
> > get_cleanup_mode such that it uses a lookup table to find the mappings
> > between string and enum and then using the same LUT in reverse to define
> > describe_cleanup_mode.
> >
> > Reviewed-by: Eric Sunshine <[email-masked]>
> > Reviewed-by: Junio C Hamano <[email-masked]>
> 
> These two Reviewed-by: lines should be dropped for a couple reasons.
> 
> First, neither Junio nor I reviewed _this_ version of the patch.
> 
> Second, a Reviewed-by: is given explicitly (not taken). When a
> reviewer has thoroughly read and understood a patch and considers it
> problem-free, he or she may say explicitly "Reviewed-by: <me>",
> stating satisfaction that the patch seems worthy of inclusion in the
> project. If he sees fit, Junio may then pick up that Reviewed-by: at
> the time he queues the patch in his tree.

My mistake, I misunderstood the purpose of the Reviewed-by tag.

I was using it to give credit for changes that I incorporated in
response to a review. In addition, it was helpful for bookkeeping who
suggested what where.

Would a Helped-by tag be more appropriate in this situation?

Thanks,

Denton

> 
> > Signed-off-by: Ramsay Jones <[email-masked]>
> > Signed-off-by: Denton Liu <[email-masked]>