Skip to content

Significant Updates#4

Open
nHurD wants to merge 16 commits intomigueldeicaza:masterfrom
nHurD:master
Open

Significant Updates#4
nHurD wants to merge 16 commits intomigueldeicaza:masterfrom
nHurD:master

Conversation

@nHurD
Copy link
Copy Markdown

@nHurD nHurD commented Dec 2, 2010

Miguel,

I did some slight refactoring of the code to facilitate support for the publish and subscribe commands that are available in Redis 2 and above. I also added a couple of extension methods that will serialize (or deserialize) objects to (or from) a given key in redis. I also created C# classes that represent sets and lists within redis.

Thanks,
Jonathan

Jonathan R. Steele and others added 16 commits September 26, 2010 17:02
I still need to add code for psubscribe, and find a method to handle
wildcards
I still need to work on cleaning up once Dispose() is called. I'll
eventually break everything out into separate files for maintainability
also...
Also started creating extension methods for getting and setting of
serializable objects
Also starte reimplementing Set commands
@migueldeicaza
Copy link
Copy Markdown
Owner

I looked through the changes and I noticed some significant changes in the refactoring.

Usually it is best if you separate "refactoring" from "feature development" so they can be reviewed independently. For instance, the file "redis.cs" is gone, and the code has gone elsewhere. Now, I could sit down and compare each line and figure out what you did, but it would be best if you describe to me one-by-one all of the changes that are included in this patch.

You did split redis.cs into a few files, and I would like to have an explanation of the rationale for the split. I see that the Redis class is now a front-end to RedisBase, why were some methods kept in RedisBase and why could the other ones not be added there and were left in Redis?

That sort of thing.

In general, smaller commits, with small explanations of the changes are easier to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants