Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor config #1008

Merged
merged 7 commits into from May 9, 2013
Merged

Refactor config #1008

merged 7 commits into from May 9, 2013

Conversation

chelseakomlo
Copy link

  • Moved Redis details from Config#redis to Backend#connect to centralize Redis information to the Backend.
  • Associated with this task

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.56%) when pulling 86ee172 on chelseakomlo:refactor_config into 9f93ea3 on resque:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.56%) when pulling 7e3c5f7 on chelseakomlo:refactor_config into 9f93ea3 on resque:master.

@ecoffey
Copy link
Member

ecoffey commented May 7, 2013

Looks good to me! 👍, @steveklabnik?

@chelseakomlo
Copy link
Author

Thank you! I'm going to keep working on isolating Redis to Backend- let me know if this commit is on the right track!

steveklabnik
steveklabnik reviewed May 8, 2013
View changes
lib/resque/config.rb
@@ -11,30 +11,11 @@ class Config
# 4. A Redis URL String 'redis://host:port'
# 5. An instance of `Redis`, `Redis::Backend`, `Redis::DistRedis`,
# or `Redis::Namespace`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This space shouldn't be added.

@steveklabnik
Copy link
Member

Love it. Just clean up those bits of whitespace, and then 👍 to merge.

@chelseakomlo
Copy link
Author

Awesome, thanks!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.56%) when pulling 80a6c86 on chelseakomlo:refactor_config into 9f93ea3 on resque:master.

@ecoffey
Copy link
Member

ecoffey commented May 9, 2013

This good to go?

@chelseakomlo chelseakomlo mentioned this pull request May 9, 2013
Closed
steveklabnik added a commit that referenced this pull request May 9, 2013
@steveklabnik
Merge pull request #1008 from chelseakomlo/refactor_config
1cdaae1 
Refactor config
@steveklabnik steveklabnik merged commit 1cdaae1 into resque:master May 9, 2013
@steveklabnik
Copy link
Member

<3 <3 <3

@fmendez
Copy link

fmendez commented May 9, 2013

Out curiosity, why remove the pry gem from the development group? Is a quite useful gem to have there IMHO.

@steveklabnik
Copy link
Member

Pry is a personal choice (that for example, I don't use) so forcing it on everyone isn't cool.

@ecoffey
Copy link
Member

ecoffey commented May 10, 2013

Congrats @chelseakomlo! 🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@chelseakomlo @coveralls @ecoffey @steveklabnik @fmendez