git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael G Schwern <schwern@pobox.com>
To: Eric Wong <normalperson@yhbt.net>
Cc: git@vger.kernel.org, gitster@pobox.com, robbat2@gentoo.org,
	bwalton@artsci.utoronto.ca, Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.
Date: Wed, 25 Jul 2012 15:39:46 -0700	[thread overview]
Message-ID: <501075B2.8090205@pobox.com> (raw)
In-Reply-To: <20120725212418.GA17494@dcvr.yhbt.net>

On 2012.7.25 2:24 PM, Eric Wong wrote:
> Please keep Jonathan Cc:-ed, he's been very helpful with this series
> (and very helpful in general :)

I will try.


>> +use Test::More 'no_plan';
> 
> Didn't we agree to use done_testing()?   Perhaps (as you suggested) with
> a private copy of Test::More?  It's probably easier to start using
> done_testing() earlier rather than later.

Yes, we agreed done_testing is the way forward.  Given how much work I've had
to do to get even basic patches in I decided to ditch anything extra.  That
includes adding a t/lib and I didn't want to make it silently depend on an
upgraded Test::More either.

There's not much difference if we do it later.  Switching to done_testing is
trivial.  I'd like to get the big class extractions in so code stops shifting
around, and worry about the minutia of test plans later.  If it happens before
I get to it, great!

PS  Those t/Git-SVN/ tests are not tied into the normal testing process.  I
felt writing the tests now was important and they could be integrated into the
test suite later.


>> +BEGIN {
>> +    # Override exit at BEGIN time before Git::SVN::Utils is loaded
>> +    # so it will see our local exit later.
>> +    *CORE::GLOBAL::exit = sub(;$) {
>> +        return @_ ? CORE::exit($_[0]) : CORE::exit();
>> +    };
>> +}
> 
> For new code related to git-svn, please match the existing indentation
> style (tabs) prevalent in git-svn.  Most of the Perl found in git also
> uses tabs for indentation.

About that.  I followed kernel style in existing code, but felt that new code
would do better to follow Perl style.  The existing Perl code mixes tabs and
spaces, so I felt it wasn't a strongly held style.  You'll get more Perl
programmers to work on the Perl code by following Perl style in the Perl code
rather than kernel style.

Alternatively, how about allowing emacs/vim configuration comments?  The
Kernel coding style doesn't allow them, how do you folks feel?  Then people
don't have to guess the style and reconfigure their editor, their editor will
do it for them.

The important thing is to have one less special thing a new-to-your-project
Perl programmer has to do.


-- 
ROCKS FALL! EVERYONE DIES!
	http://www.somethingpositive.net/sp05032002.shtml

  reply	other threads:[~2012-07-25 22:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-25  6:01 Move Git::SVN into its own .pm file Michael G. Schwern
2012-07-25  6:01 ` [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN Michael G. Schwern
2012-07-25 21:24   ` Eric Wong
2012-07-25 22:39     ` Michael G Schwern [this message]
2012-07-25 23:08       ` Eric Wong
2012-07-26  0:01         ` Michael G Schwern
2012-07-26  0:25           ` Eric Wong
2012-07-26  0:26           ` Jonathan Nieder
2012-07-25  6:01 ` [PATCH 2/4] Prepare Git::SVN for extraction into its own file Michael G. Schwern
2012-07-25  6:01 ` [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN Michael G. Schwern
2012-07-25 21:15 ` Move Git::SVN into its own .pm file Jonathan Nieder
2012-07-25 21:56   ` Michael G Schwern
  -- strict thread matches above, loose matches on Subject: below --
2012-07-26 23:22 Extract Git::SVN from git-svn, take 2 Michael G. Schwern
2012-07-26 23:22 ` [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN Michael G. Schwern
2012-07-27  5:18   ` Junio C Hamano
2012-07-27  8:19     ` Michael G Schwern
2012-07-27 11:34     ` Eric Wong

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

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

  git send-email \
    --in-reply-to=501075B2.8090205@pobox.com \
    --to=schwern@pobox.com \
    --cc=bwalton@artsci.utoronto.ca \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=normalperson@yhbt.net \
    --cc=robbat2@gentoo.org \
    /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.
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 NNTP newsgroup(s).