Skip to content
This repository has been archived by the owner on Sep 22, 2020. It is now read-only.

Store token in Redis and Memcache or other objects #5

Merged
merged 4 commits into from
Apr 13, 2015

Conversation

dmvieira
Copy link
Contributor

@dmvieira dmvieira commented Apr 6, 2015

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
Copy link
Contributor

scorphus commented Apr 7, 2015

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

@dmvieira
Copy link
Contributor Author

dmvieira commented Apr 7, 2015

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

@scorphus
Copy link
Contributor

scorphus commented Apr 7, 2015

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 😃

@dmvieira
Copy link
Contributor Author

dmvieira commented Apr 7, 2015

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: 😃]


Reply to this email directly or view it on GitHub
#5 (comment).

@scorphus
Copy link
Contributor

scorphus commented Apr 7, 2015

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
Copy link
Collaborator

ricobl commented Apr 7, 2015

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
Copy link
Contributor Author

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

@scorphus
Copy link
Contributor

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

@dmvieira
Copy link
Contributor Author

I'll try a bit more

@dmvieira dmvieira reopened this Apr 11, 2015
@dmvieira
Copy link
Contributor Author

Please take a look @scorphus

ricobl added a commit that referenced this pull request Apr 13, 2015
Store token in Redis and Memcache or other objects
@ricobl ricobl merged commit 287938c into globocom:master Apr 13, 2015
@scorphus
Copy link
Contributor

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!” 👍

@ricobl
Copy link
Collaborator

ricobl commented Apr 13, 2015

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

The 0.5.0 version is now released on PyPI.

@dmvieira
Copy link
Contributor Author

\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
#5 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants