Re: [PATCH v3 00/28] Follow perlcritic's recommandations
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
Eric Sunshine writes: > On Sun, Jun 9, 2013 at 6:22 PM, Célestin Matte > 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
Re: [PATCH v3 00/28] Follow perlcritic's recommandations
On Sun, Jun 9, 2013 at 6:22 PM, Célestin Matte 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
[PATCH v3 00/28] Follow perlcritic's recommandations
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