search

globocom / alf

Python OAuth2 Client
MIT License
32 stars 12 forks source link

Store token in Redis and Memcache or other objects #5

Closed dmvieira closed 9 years ago

dmvieira commented 9 years ago

Now passing an object with 'get' and 'set' attributes you can store or retrieve a token.

This object can be a Redis, Memcache or your custom object.

scorphus commented 9 years ago

How about squashing all these commits that introduce a single new feature into a single one commit, that stands alone?

dmvieira commented 9 years ago

Ok, ok, but why? Just to understand...

scorphus commented 9 years ago

For many reasons. There is a couple of commits there that just represent a mistake being corrected. They don't really belong to this feature. Other commits only represent you getting things to work or to look alright (such as docs). In other words: one-commit-per-feature keeps the history clean. I don't (nor anyone else does) care if in the middle of your work you changed your mind or found something not right or that could be done better. It's just that mantra “Commit Often, Perfect Later, Publish Once”.

But, equally important, it helps code reviewing and it really, really helps bisecting (hence debugging).

I hope you agree with me :smiley:

dmvieira commented 9 years ago

Ohhhh ok! I agree :D

Please close this PR. I'll make it better

2015-04-07 11:01 GMT-03:00 Pablo Aguiar notifications@github.com:

For many reasons. There are a couple of commits there that just represent a mistake being corrected. They don't really belong to this feature. Other commits only represent you getting things to work or to look alright (such as docs). In other words: one-commit-per-feature keeps the history clean. I don't (nor anyone else) care if in the middle of you work you changed your mind or found something not right or that could be done better. It's just that mantra “Commit Often, Perfect Later, Publish Once”.

But, equally important, it helps code reviewing and it really, really helps bisecting (hence debugging).

I hope you agree with me [image: :smiley:]

— Reply to this email directly or view it on GitHub https://github.com/globocom/alf/pull/5#issuecomment-90567321.

scorphus commented 9 years ago

Great!

Luckily, there's no need to close/reopen, just force-push to this same branch (unfortunately it's master, but fortunately it's your master) and this PR will be updated (kudos to GitHub).

ricobl commented 9 years ago

Also, it would be nice to avoid the dependency of redis. Or at least make it optional. People may want to use different services like memcached or filesystem.

The default implementation could have a simple, in-memory storage. Additional storages could be available as external packages or in a contrib module.

dmvieira commented 9 years ago

Tests could be improved (but I don't know how) and squashing is returning a lot of conflicts... But it's working :)

scorphus commented 9 years ago

I can drop by and give you a hand, if you'd like :)

dmvieira commented 9 years ago

I'll try a bit more

dmvieira commented 9 years ago

Please take a look @scorphus

scorphus commented 9 years ago

I didn't have the opportunity to look into your latest changes but, had I done so prior to the merge, all I'd have said is “Nice job, @dmvieira! Thanks!” :+1:

ricobl commented 9 years ago

Thanks for the contribution @dmvieira and @scorphus for the review!

The 0.5.0 version is now released on PyPI.

dmvieira commented 9 years ago

\o/ Thanks for all help ;)

2015-04-13 11:42 GMT-03:00 Enrico Batista da Luz notifications@github.com:

Thanks for the contribution @dmvieira https://github.com/dmvieira and @scorphus https://github.com/scorphus for the review!

The 0.5.0 version is now released on PyPI.

— Reply to this email directly or view it on GitHub https://github.com/globocom/alf/pull/5#issuecomment-92385578.