git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: "Célestin Matte" <celestin.matte@ensimag.fr>,
	"Git List" <git@vger.kernel.org>,
	benoit.person@ensimag.fr
Subject: Re: [PATCH v3 00/28] Follow perlcritic's recommandations
Date: Mon, 10 Jun 2013 10:46:40 +0200	[thread overview]
Message-ID: <vpq7gi2qrnz.fsf@anie.imag.fr> (raw)
In-Reply-To: <CAPig+cR1=32TwatmTdVBDnkpkhwtUNyKL_Z9f=V_FPtt_Y-xiA@mail.gmail.com> (Eric Sunshine's message of "Sun, 9 Jun 2013 21:09:52 -0400")

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sun, Jun 9, 2013 at 6:22 PM, Célestin Matte
> <celestin.matte@ensimag.fr> wrote:
>> Changes with v2:
>> - Remove patch [02/22] about using the Readonly module
>> - Split commit [07/22] into 5 different ones
>
> This was easier to review after being split. Thanks.
>
>> - Split commit [14/22] into 2 different ones
>> - Patch [17/22] was *not* split: tell me if it is necessary
>
> [now patch 22/28]
>
> You, Matthieu, and Junio should decide, but I again found it
> time-consuming and onerous to review with all the changes mashed
> together.

I agree that it would have been better to split the patches in v1, but
now that we've already spent time reviewing it, it seems unecessary to
spend more time splitting and re-reviewing.

I went through the series once more, all my remarks are minor. I'm OK
with the series as it is (i.e. perhaps it's time to say "stop the
bikeshedding and start coding real stuff" ;-) ). As a reminder: Celestin
is in a school project that ends in a week. The goal is both to be
productive and to learn stuff.

In any case, thanks a lot for your review, Eric.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

  reply	other threads:[~2013-06-10  8:46 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-09 22:22 [PATCH v3 00/28] Follow perlcritic's recommandations Célestin Matte
2013-06-09 22:22 ` [PATCH v3 01/28] git-remote-mediawiki: Make a regexp clearer Célestin Matte
2013-06-09 22:22 ` [PATCH v3 02/28] git-remote-mediawiki: Move "use warnings;" before any instruction Célestin Matte
2013-06-09 22:22 ` [PATCH v3 03/28] git-remote-mediawiki: Replace :utf8 by :encoding(UTF-8) Célestin Matte
2013-06-09 22:22 ` [PATCH v3 04/28] git-remote-mediawiki: Always end a subroutine with a return Célestin Matte
2013-06-09 22:22 ` [PATCH v3 05/28] git-remote-mediawiki: Move a variable declaration at the top of the code Célestin Matte
2013-06-09 22:22 ` [PATCH v3 06/28] git-remote-mediawiki: Change syntax of map calls Célestin Matte
2013-06-09 22:22 ` [PATCH v3 07/28] git-remote-mediawiki: Rewrite unclear line of instructions Célestin Matte
2013-06-11 15:04   ` Junio C Hamano
2013-06-11 16:35     ` Matthieu Moy
2013-06-11 18:09       ` Junio C Hamano
2013-06-11 20:13         ` Célestin Matte
2013-06-11 21:55           ` Junio C Hamano
2013-06-09 22:22 ` [PATCH v3 08/28] git-remote-mediawiki: Remove useless regexp modifier (m) Célestin Matte
2013-06-09 22:22 ` [PATCH v3 09/28] git-remote-mediawiki: Change the behaviour of a split Célestin Matte
2013-06-10  8:29   ` Matthieu Moy
2013-06-09 22:22 ` [PATCH v3 10/28] git-remote-mediawiki: Change separator of some regexps Célestin Matte
2013-06-09 22:22 ` [PATCH v3 11/28] git-remote-mediawiki: Change style in a regexp Célestin Matte
2013-06-09 22:22 ` [PATCH v3 12/28] " Célestin Matte
2013-06-09 22:22 ` [PATCH v3 13/28] git-remote-mediawiki: Add newline in the end of die() error messages Célestin Matte
2013-06-09 22:22 ` [PATCH v3 14/28] git-remote-mediawiki: Change the name of a variable Célestin Matte
2013-06-09 22:22 ` [PATCH v3 15/28] git-remote-mediawiki: Turn double-negated expressions into simple expressions Célestin Matte
2013-06-09 22:22 ` [PATCH v3 16/28] git-remote-mediawiki: Remove unused variable $entry Célestin Matte
2013-06-09 22:22 ` [PATCH v3 17/28] git-remote-mediawiki: Rename a variable ($last) which has the name of a keyword Célestin Matte
2013-06-09 22:22 ` [PATCH v3 18/28] git-remote-mediawiki: Assign a variable as undef and make proper indentation Célestin Matte
2013-06-09 22:22 ` [PATCH v3 19/28] git-remote-mediawiki: Check return value of open Célestin Matte
2013-06-09 22:22 ` [PATCH v3 20/28] git-remote-mediawiki: remove import of unused open2 Célestin Matte
2013-06-09 22:22 ` [PATCH v3 21/28] git-remote-mediawiki: Put long code into a subroutine Célestin Matte
2013-06-11 15:42   ` Junio C Hamano
2013-06-11 22:24     ` Célestin Matte
2013-06-09 22:22 ` [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style Célestin Matte
2013-06-10  0:50   ` Eric Sunshine
2013-06-10 10:48     ` Célestin Matte
2013-06-10  8:37   ` Matthieu Moy
2013-06-10 11:00     ` Célestin Matte
2013-06-10 16:41     ` Junio C Hamano
2013-06-10 17:18       ` Benoît Person
2013-06-10 17:30         ` Matthieu Moy
2013-06-10 19:01           ` Junio C Hamano
2013-06-09 22:22 ` [PATCH v3 23/28] git-remote-mediawiki: Brace file handles for print for more clarity Célestin Matte
2013-06-09 22:22 ` [PATCH v3 24/28] git-remote-mediawiki: Replace "unless" statements with negated "if" statements Célestin Matte
2013-06-09 22:22 ` [PATCH v3 25/28] git-remote-mediawiki: Don't use quotes for empty strings Célestin Matte
2013-06-09 22:22 ` [PATCH v3 26/28] git-remote-mediawiki: Put non-trivial numeric values in constants Célestin Matte
2013-06-09 22:22 ` [PATCH v3 27/28] git-remote-mediawiki: Fix a typo ("mediwiki" instead of "mediawiki") Célestin Matte
2013-06-09 22:22 ` [PATCH v3 28/28] git-remote-mediawiki: Clearly rewrite double dereference Célestin Matte
2013-06-10  1:09 ` [PATCH v3 00/28] Follow perlcritic's recommandations Eric Sunshine
2013-06-10  8:46   ` Matthieu Moy [this message]
2013-06-11 12:54     ` Célestin Matte

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=vpq7gi2qrnz.fsf@anie.imag.fr \
    --to=matthieu.moy@grenoble-inp.fr \
    --cc=benoit.person@ensimag.fr \
    --cc=celestin.matte@ensimag.fr \
    --cc=git@vger.kernel.org \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).