Re: [PATCH/RFC v2 5/5] send-email: refactor address list process

2015-06-08 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:

 Simplify code by creating a function to transform list of email lists
 (comma separated, with aliases ...)  into a simple list of valid email
 addresses.

 I would have found the series easier to read if this refactoring came
 earlier (and then PATCH 2/5 would fix the bug as a positive side effect
 of the refactoring).

I agree that doing 5/5 sooner would make 4/5 a lot clearer.  

Introducing the helper of 5/5 before 2/5 happens, and then replacing
two calls to validate-address-list with process-address-list would
hide the nature of the change, i.e. fixing a bug, so it is better to
see it done before the refactoring of 5/5, provided if it is indeed
a bug that these were not expanded.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v2 5/5] send-email: refactor address list process

2015-06-08 Thread Matthieu Moy
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:

 Simplify code by creating a function to transform list of email lists
 (comma separated, with aliases ...)  into a simple list of valid email
 addresses.

I would have found the series easier to read if this refactoring came
earlier (and then PATCH 2/5 would fix the bug as a positive side effect
of the refactoring). I think it's too late to change this, though.

  I'm not sure about the name of the function...

process_address_list() sounds good to me.

The whole series looks good to me now.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v2 5/5] send-email: refactor address list process

2015-06-08 Thread Matthieu Moy
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:

 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes:
 
  Simplify code by creating a function to transform list of email lists
  (comma separated, with aliases ...)  into a simple list of valid email
  addresses.
 
 I would have found the series easier to read if this refactoring came
 earlier (and then PATCH 2/5 would fix the bug as a positive side effect
 of the refactoring). I think it's too late to change this, though.

 Why is it to late? I can still change it if necessary.

To me, the series is ready now, and I don't think re-rolling it would be
a good time investment. Plus, I spent time reviewing this series and
with my proposal I'd need to review a relatively different one.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html