Where to send patches to the Perl Client

9 views
Skip to first unread message

jza...@gmail.com

unread,
Sep 10, 2009, 6:10:26 PM9/10/09
to Redis DB
I've noticed that the Perl client doesn't provide a way to specify the
host:port pair when creating a new object and am patching it to fix
that.

Where is the best place to send patches? The docs in the included
Perl module are, sparse, at best.

Thanks,

Jeremy

Salvatore Sanfilippo

unread,
Sep 10, 2009, 6:55:59 PM9/10/09
to redi...@googlegroups.com

Hello,

I think the best thing to do is to contact the author:
http://www.rot13.org/~dpavlin/
I'm not sure he is subscribed to this list.

Cheers,
Salvatore


--
Salvatore 'antirez' Sanfilippo
http://invece.org

"Once you have something that grows faster than education grows,
you’re always going to get a pop culture.", Alan Kay

jza...@gmail.com

unread,
Sep 10, 2009, 7:24:24 PM9/10/09
to Redis DB
Thanks, I'll contact him (assuming I can find his email address
somewhere on that site!).

Jeremy

On Sep 10, 3:55 pm, Salvatore Sanfilippo <anti...@gmail.com> wrote:
> On Fri, Sep 11, 2009 at 12:10 AM, Jer...@Zawodny.com <jzaw...@gmail.com> wrote:
>
> > I've noticed that the Perl client doesn't provide a way to specify the
> > host:port pair when creating a new object and am patching it to fix
> > that.
>
> > Where is the best place to send patches?  The docs in the included
> > Perl module are, sparse, at best.
>
> Hello,
>
> I think the best thing to do is to contact the author:http://www.rot13.org/~dpavlin/
> I'm not sure he is subscribed to this list.
>
> Cheers,
> Salvatore
>
> --
> Salvatore 'antirez' Sanfilippohttp://invece.org

Dobrica Pavlinusic

unread,
Sep 11, 2009, 6:03:46 AM9/11/09
to redi...@googlegroups.com
On Thu, Sep 10, 2009 at 04:24:24PM -0700, Jer...@Zawodny.com wrote:
> Thanks, I'll contact him (assuming I can find his email address
> somewhere on that site!).

I allready got patch which allows setup of redis server using enviroment
variable, so I'm about to update client library to use it.

I will also add my e-mail address dpavlin(at)rot13(dot)org into module
documentation somewhere :-)

> On Sep 10, 3:55 pm, Salvatore Sanfilippo <anti...@gmail.com> wrote:
> > On Fri, Sep 11, 2009 at 12:10 AM, Jer...@Zawodny.com <jzaw...@gmail.com> wrote:
> >
> > > I've noticed that the Perl client doesn't provide a way to specify the
> > > host:port pair when creating a new object and am patching it to fix
> > > that.
> >
> > > Where is the best place to send patches?  The docs in the included
> > > Perl module are, sparse, at best.
> >
> > Hello,
> >
> > I think the best thing to do is to contact the author:http://www.rot13.org/~dpavlin/
> > I'm not sure he is subscribed to this list.
> >
> > Cheers,
> > Salvatore
> >
> > --
> > Salvatore 'antirez' Sanfilippohttp://invece.org
> >
> > "Once you have something that grows faster than education grows,
> > you’re always going to get a pop culture.", Alan Kay
> >
>

--
Dobrica Pavlinusic 2share!2flame dpa...@rot13.org
Unix addict. Internet consultant. http://www.rot13.org/~dpavlin

jza...@gmail.com

unread,
Sep 11, 2009, 1:36:44 PM9/11/09
to Redis DB
Excellent... Ideally you'd allow one to pass arguments to the
constructor too...

my $r = Redis->new(host => 'foo', port => 54321);

Jeremy
> Dobrica Pavlinusic               2share!2flame            dpav...@rot13.org

jza...@gmail.com

unread,
Sep 11, 2009, 2:22:20 PM9/11/09
to Redis DB
BTW, do you keep your code in a public repo anywhere? Or should I
just keep an eye on the main redis github repository?

Thanks again,

Jeremy

Dobrica Pavlinusic

unread,
Sep 12, 2009, 11:18:51 AM9/12/09
to redi...@googlegroups.com
On Fri, Sep 11, 2009 at 11:22:20AM -0700, Jer...@Zawodny.com wrote:
>
> BTW, do you keep your code in a public repo anywhere? Or should I
> just keep an eye on the main redis github repository?

Upstream repository is http://svn.rot13.org/index.cgi/Redis/

> On Sep 11, 10:36 am, "Jer...@Zawodny.com" <jzaw...@gmail.com> wrote:
> > Excellent...  Ideally you'd allow one to pass arguments to the
> > constructor too...
> >
> > my $r = Redis->new(host => 'foo', port => 54321);

Hmmm... I would prefer something like

my $r = Redis->new( server => 'localhost:6379', debug => 1 );

which would enable environment variables to be REDIS_SERVER and
REDIS_DEBUG.

It seems to me that having two parameters is a bit too verbose, and
could introduce errors in configuration (e.g. what happens if you specify
just host? Should it use default port?). Does that make sense?

This would also change behavior of Redis.pm which now always re-use
single connection to server, but that is good thing for concurrency so
I'm all for it :-)

Hack, I even implemented it while writing this e-mail...

Version 0.0801 is in my svn repository and if there isn't any
objections to new API I will also push it to CPAN so documentation
won't lie any more about pointers to bug tracker...

--
Dobrica Pavlinusic 2share!2flame dpa...@rot13.org

jza...@gmail.com

unread,
Sep 12, 2009, 3:24:24 PM9/12/09
to Redis DB
On Sep 12, 8:18 am, Dobrica Pavlinusic <dpav...@gmail.com> wrote:
> On Fri, Sep 11, 2009 at 11:22:20AM -0700, Jer...@Zawodny.com wrote:
>
> > BTW, do you keep your code in a public repo anywhere?  Or should I
> > just keep an eye on the main redis github repository?
>
> Upstream repository ishttp://svn.rot13.org/index.cgi/Redis/

Ah, excellent. That'll make it easier to generate patches. :-)

> > On Sep 11, 10:36 am, "Jer...@Zawodny.com" <jzaw...@gmail.com> wrote:
> > > Excellent...  Ideally you'd allow one to pass arguments to the
> > > constructor too...
>
> > > my $r = Redis->new(host => 'foo', port => 54321);
>
> Hmmm... I would prefer something like
>
> my $r = Redis->new( server => 'localhost:6379', debug => 1 );
>
> which would enable environment variables to be REDIS_SERVER and
> REDIS_DEBUG.

Sure, I'm not too concerned with *how* you specify the host and port,
as long as you *can* do it.

> It seems to me that having two parameters is a bit too verbose, and
> could introduce errors in configuration (e.g. what happens if you specify
> just host? Should it use default port?). Does that make sense?

If you did go that route, yes, I'd use localhost as the default host
and the normal port as the default port. In our situation, I'm
planning to run multiple redis servers per host, each on a different
port.

> This would also change behavior of Redis.pm which now always re-use
> single connection to server, but that is good thing for concurrency so
> I'm all for it :-)

Yes.

> Version 0.0801 is in my svn repository and if there isn't any
> objections to new API I will also push it to CPAN so documentation
> won't lie any more about pointers to bug tracker...

Great!

The other thing I did on my copy was to fix some of the tests and the
main code to remove the requirement to have Data::Dump installed. It
turns out that's really only necessary for one test, yet several tests
'use' it. Similarly, one of the sub-modules 'use'd it without ever
calling it. And the main module needs it only for debug purposes.

But maybe I'll just bite the bullet and get Data::Dump installed on
all our servers. Unless you want to use Data::Dumper instead. :-)

Thanks!

Jeremy

Dobrica Pavlinusic

unread,
Sep 14, 2009, 7:52:04 AM9/14/09
to redi...@googlegroups.com
On Sat, Sep 12, 2009 at 12:24:24PM -0700, Jer...@Zawodny.com wrote:
> The other thing I did on my copy was to fix some of the tests and the
> main code to remove the requirement to have Data::Dump installed. It
> turns out that's really only necessary for one test, yet several tests
> 'use' it. Similarly, one of the sub-modules 'use'd it without ever
> calling it. And the main module needs it only for debug purposes.
>
> But maybe I'll just bite the bullet and get Data::Dump installed on
> all our servers. Unless you want to use Data::Dumper instead. :-)

I really like Data::Dump output, but not so much to introduce non-core
dependency just for debugging... I replaced it with Data::Dumper.

I also created fork on github of redis in which I commit
revision-by-revision my changes to subversion so github users can take
advantage of changes. I also sent pull request to Salvatore so it should
appear on github soon, I hope.

jza...@gmail.com

unread,
Sep 14, 2009, 12:03:03 PM9/14/09
to Redis DB
On Sep 14, 4:52 am, Dobrica Pavlinusic <dpav...@gmail.com> wrote:
> On Sat, Sep 12, 2009 at 12:24:24PM -0700, Jer...@Zawodny.com wrote:
> > The other thing I did on my copy was to fix some of the tests and the
> > main code to remove the requirement to have Data::Dump installed.  It
> > turns out that's really only necessary for one test, yet several tests
> > 'use' it.  Similarly, one of the sub-modules 'use'd it without ever
> > calling it.  And the main module needs it only for debug purposes.
>
> > But maybe I'll just bite the bullet and get Data::Dump installed on
> > all our servers.  Unless you want to use Data::Dumper instead. :-)
>
> I really like Data::Dump output, but not so much to introduce non-core
> dependency just for debugging... I replaced it with Data::Dumper.
>
> I also created fork on github of redis in which I commit
> revision-by-revision my changes to subversion so github users can take
> advantage of changes. I also sent pull request to Salvatore so it should
> appear on github soon, I hope.

Fantastic, thanks!

Jeremy
Reply all
Reply to author
Forward
0 new messages