Re: [PATCH v3 00/28] Follow perlcritic's recommandations

2013-06-11 Thread Célestin Matte
So, do I send a last version of the patch? What is left is quick fix:
- removing whitespace in [18/28]
- typo in [09/28]
- better line split in [22/28]
I already fixed first two problems, so it would be done rapidly.

-- 
Célestin Matte
--
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 v3 00/28] Follow perlcritic's recommandations

2013-06-10 Thread Matthieu Moy
Eric Sunshine sunsh...@sunshineco.com writes:

 On Sun, Jun 9, 2013 at 6:22 PM, Célestin Matte
 celestin.ma...@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/
--
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


[PATCH v3 00/28] Follow perlcritic's recommandations

2013-06-09 Thread Célestin Matte
Changes with v2:
- Remove patch [02/22] about using the Readonly module
- Split commit [07/22] into 5 different ones
- Split commit [14/22] into 2 different ones
- Patch [17/22] was *not* split: tell me if it is necessary
- Remove wrong change in patch [22/22]

Patch about fixing a bug in a regexp (with has been renames Make a regexp
clearer), which had been sent as a separate patch, is reintegrated into this
series of patches.

Changes with v1:
- split first commit into 6 different commits
- remove commit [17/18] about moving open() call
- took every other comment into account

Célestin Matte (28):
  git-remote-mediawiki: Make a regexp clearer
  git-remote-mediawiki: Move use warnings; before any instruction
  git-remote-mediawiki: Replace :utf8 by :encoding(UTF-8)
  git-remote-mediawiki: Always end a subroutine with a return
  git-remote-mediawiki: Move a variable declaration at the top of the
code
  git-remote-mediawiki: Change syntax of map calls
  git-remote-mediawiki: Rewrite unclear line of instructions
  git-remote-mediawiki: Remove useless regexp modifier (m)
  git-remote-mediawiki: Change the behaviour of a split
  git-remote-mediawiki: Change separator of some regexps
  git-remote-mediawiki: Change style in a regexp
  git-remote-mediawiki: Change style in a regexp
  git-remote-mediawiki: Add newline in the end of die() error messages
  git-remote-mediawiki: Change the name of a variable
  git-remote-mediawiki: Turn double-negated expressions into simple
expressions
  git-remote-mediawiki: Remove unused variable $entry
  git-remote-mediawiki: Rename a variable ($last) which has the name of
a keyword
  git-remote-mediawiki: Assign a variable as undef and make proper
indentation
  git-remote-mediawiki: Check return value of open
  git-remote-mediawiki: remove import of unused open2
  git-remote-mediawiki: Put long code into a subroutine
  git-remote-mediawiki: Modify strings for a better coding-style
  git-remote-mediawiki: Brace file handles for print for more clarity
  git-remote-mediawiki: Replace unless statements with negated if
statements
  git-remote-mediawiki: Don't use quotes for empty strings
  git-remote-mediawiki: Put non-trivial numeric values in constants.
  git-remote-mediawiki: Fix a typo (mediwiki instead of mediawiki)
  git-remote-mediawiki: Clearly rewrite double dereference

 contrib/mw-to-git/git-remote-mediawiki.perl |  534 +++
 1 file changed, 288 insertions(+), 246 deletions(-)

-- 
1.7.9.5

--
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 v3 00/28] Follow perlcritic's recommandations

2013-06-09 Thread Eric Sunshine
On Sun, Jun 9, 2013 at 6:22 PM, Célestin Matte
celestin.ma...@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.

 - Remove wrong change in patch [22/22]

 Patch about fixing a bug in a regexp (with has been renames Make a regexp
 clearer), which had been sent as a separate patch, is reintegrated into this
 series of patches.

Overall, this series was more pleasant and easy to read than previous
versions. With the exception of minor comments, questions, and the
whitespace noise in 22/28, I did not spot any issues worth mentioning
in the remainder of the series.
--
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