git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Célestin Matte" <celestin.matte@ensimag.fr>
To: git@vger.kernel.org
Cc: benoit.person@ensimag.fr, matthieu.moy@grenoble-inp.fr,
	"Célestin Matte" <celestin.matte@ensimag.fr>
Subject: [PATCH 00/18] git-remote-mediawiki: Follow perlcritic's recommandations
Date: Thu,  6 Jun 2013 21:34:05 +0200	[thread overview]
Message-ID: <1370547263-13558-1-git-send-email-celestin.matte@ensimag.fr> (raw)

Hi,
This series of commits intends to follow most of perlcritic's recommandations
in order to make the code more maintainable and readable.
I followed most recommandations from level 5 (most critical ones )to 2, but 
left a great part of the level 1 ones, as they were more about personal choices
of coding style and less about actually improving maintainability and preventing
bugs. Among those I did *not* take into account were:
- ValuesAndExpressions::ProhibitNoisyQuotes: use q(*) instead of '*' for quoting
  some characters
- RegularExpressions::ProhibitEnumeratedClasses: e.g., use [0-9] instead of \d
- CodeLayout::ProhibitParensWithBuiltins: don't use parentheses with builtin
  functions
- RegularExpressions::RequireExtendedFormatting, 
  RegularExpressions::RequireDotMatchAnything and
  RegularExpressions::RequireLineBoundaryMatching: use s, m and x flags for
  *every* regexp
- and some others. Run perlcritic -1 on patched file to so what is left.
Please tell me if you think some of them should be applied anyway.

On the other hand, if quite not sure of the relevance of some commits:
- Add newline in the end of die() error messages [3/18]: are die() messages
  correctly handled already?
- Place the open() call inside the do{} struct and prevent failing close
  [17/18]: current code fails to close the filehandle (try adding a 
  "or warn(..)" after the close() to check this). However, perlcritic complains
  as well if you don't put an explicit close(), which fails when you put it.
  I'm not sure of what's happening and what's the best solution for this.

I checked Documentation/CodingGuidelines and none of these modifications should
contradict it.
This is my first patch, so don't hesitate to point to me anything wrong with it.

Célestin Matte (18):
  Follow perlcritic's recommendations - level 5 and 4
  Change style of some regular expressions to make them clearer
  Add newline in the end of die() error messages
  Prevent local variable $url to have the same name as a global
    variable
  Turn double-negated expressions into simple expressions
  Remove unused variable
  Rename a variable ($last) so that it does not have the name of a
    keyword
  Explicitely assign local variable as undef and make a proper
    one-instruction-by-line indentation
  Check return value of open and remove import of unused open2
  Put long code into a submodule
  Modify strings for a better coding-style
  Brace file handles for print for more clarity
  Remove "unless" statements and replace them by negated "if"
    statements
  Don't use quotes for empty strings
  Put non-trivial numeric values (e.g., different from 0, 1 and 2) in
    constants.
  Fix a typo ("mediwiki" instead of "mediawiki")
  Place the open() call inside the do{} struct and prevent failing
    close
  Clearly rewrite double dereference

 contrib/mw-to-git/git-remote-mediawiki.perl |  558 ++++++++++++++-------------
 1 file changed, 298 insertions(+), 260 deletions(-)

-- 
1.7.9.5

             reply	other threads:[~2013-06-06 19:34 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-06 19:34 Célestin Matte [this message]
2013-06-06 19:34 ` [PATCH 01/18] Follow perlcritic's recommendations - level 5 and 4 Célestin Matte
2013-06-07  1:42   ` Eric Sunshine
2013-06-07  8:10   ` Matthieu Moy
2013-06-07 12:11     ` Célestin Matte
2013-06-07 17:43       ` Matthieu Moy
2013-06-06 19:34 ` [PATCH 02/18] Change style of some regular expressions to make them clearer Célestin Matte
2013-06-07  1:54   ` Eric Sunshine
2013-06-07  2:30     ` Junio C Hamano
2013-06-07  4:39       ` Eric Sunshine
2013-06-07  4:51         ` Junio C Hamano
2013-06-07 10:40   ` Peter Krefting
2013-06-06 19:34 ` [PATCH 03/18] Add newline in the end of die() error messages Célestin Matte
2013-06-06 19:34 ` [PATCH 04/18] Prevent local variable $url to have the same name as a global variable Célestin Matte
2013-06-06 19:34 ` [PATCH 05/18] Turn double-negated expressions into simple expressions Célestin Matte
2013-06-07  4:12   ` Eric Sunshine
2013-06-07 17:04     ` Célestin Matte
2013-06-07 20:25       ` Eric Sunshine
2013-06-07 20:32         ` Célestin Matte
2013-06-06 19:34 ` [PATCH 06/18] Remove unused variable Célestin Matte
2013-06-06 19:34 ` [PATCH 07/18] Rename a variable ($last) so that it does not have the name of a keyword Célestin Matte
2013-06-06 19:34 ` [PATCH 08/18] Explicitely assign local variable as undef and make a proper one-instruction-by- line indentation Célestin Matte
2013-06-07  1:19   ` Eric Sunshine
2013-06-07  8:18   ` Matthieu Moy
2013-06-06 19:34 ` [PATCH 09/18] Check return value of open and remove import of unused open2 Célestin Matte
2013-06-07  8:21   ` Matthieu Moy
2013-06-06 19:34 ` [PATCH 10/18] Put long code into a submodule Célestin Matte
2013-06-07  4:01   ` Eric Sunshine
2013-06-07  4:51     ` Junio C Hamano
2013-06-06 19:34 ` [PATCH 11/18] Modify strings for a better coding-style Célestin Matte
2013-06-07  4:31   ` Eric Sunshine
2013-06-06 19:34 ` [PATCH 12/18] Brace file handles for print for more clarity Célestin Matte
2013-06-06 19:34 ` [PATCH 13/18] Remove "unless" statements and replace them by negated "if" statements Célestin Matte
2013-06-07  3:41   ` Eric Sunshine
2013-06-06 19:34 ` [PATCH 14/18] Don't use quotes for empty strings Célestin Matte
2013-06-06 19:34 ` [PATCH 15/18] Put non-trivial numeric values (e.g., different from 0, 1 and 2) in constants Célestin Matte
2013-06-06 19:34 ` [PATCH 16/18] Fix a typo ("mediwiki" instead of "mediawiki") Célestin Matte
2013-06-06 19:34 ` [PATCH 17/18] Place the open() call inside the do{} struct and prevent failing close Célestin Matte
2013-06-06 21:13   ` Junio C Hamano
2013-06-06 21:30     ` Célestin Matte
2013-06-06 21:58       ` Junio C Hamano
2013-06-06 22:16         ` Célestin Matte
2013-06-06 19:34 ` [PATCH 18/18] Clearly rewrite double dereference Célestin Matte
2013-06-07  4:04   ` Eric Sunshine

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=1370547263-13558-1-git-send-email-celestin.matte@ensimag.fr \
    --to=celestin.matte@ensimag.fr \
    --cc=benoit.person@ensimag.fr \
    --cc=git@vger.kernel.org \
    --cc=matthieu.moy@grenoble-inp.fr \
    /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).