Where to send patches to the Perl Client

[jza...@gmail.com]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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