Change redis port and add in bundler/shotgun #13

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
9 participants
@qrush

qrush commented Oct 21, 2011

First of many cleanups :)

Also removed the require for Hiredis, it's not used yet so it can just be added back in once it's needed.

@qrush

This comment has been minimized.

Show comment
Hide comment
@qrush

qrush Oct 23, 2011

Any feedback on this? not sure why it hasn't been merged. trying to make it easier for others to contribute!

qrush commented Oct 23, 2011

Any feedback on this? not sure why it hasn't been merged. trying to make it easier for others to contribute!

@PabloC

This comment has been minimized.

Show comment
Hide comment
@PabloC

PabloC Oct 23, 2011

+1 for this! Thanks Antirez and qrush!

PabloC commented Oct 23, 2011

+1 for this! Thanks Antirez and qrush!

@ThroughTheNet

This comment has been minimized.

Show comment
Hide comment
@ThroughTheNet

ThroughTheNet Oct 23, 2011

Contributor

bundler support is essential, but this gemfile will load shotgun in the production environment, which is undesirable. Change the line to:

gem 'shotgun', :group => :development

TBH I don't use shotgun as it's too slow, and a lot of other people do the same for the same reason, so maybe it shouldn't be included at all? People generally make their own decisions on these things.

Contributor

ThroughTheNet commented Oct 23, 2011

bundler support is essential, but this gemfile will load shotgun in the production environment, which is undesirable. Change the line to:

gem 'shotgun', :group => :development

TBH I don't use shotgun as it's too slow, and a lot of other people do the same for the same reason, so maybe it shouldn't be included at all? People generally make their own decisions on these things.

@sgoodwin

This comment has been minimized.

Show comment
Hide comment
@sgoodwin

sgoodwin Oct 23, 2011

Haha. I was just about to issue a similar pull request myself.

Haha. I was just about to issue a similar pull request myself.

@antirez

This comment has been minimized.

Show comment
Hide comment
@antirez

antirez Oct 25, 2011

Owner

Please can you rebase this? I'll merge ASAP. Thanks.

Owner

antirez commented Oct 25, 2011

Please can you rebase this? I'll merge ASAP. Thanks.

@antirez

This comment has been minimized.

Show comment
Hide comment
@antirez

antirez Oct 25, 2011

Owner

p.s. +1 for not including shotgun. I use it but that's up to the developer, and we should just specify the essential gems to run the code IMHO.

Owner

antirez commented Oct 25, 2011

p.s. +1 for not including shotgun. I use it but that's up to the developer, and we should just specify the essential gems to run the code IMHO.

@PabloC

This comment has been minimized.

Show comment
Hide comment
@PabloC

PabloC Oct 25, 2011

and what about support for 1.9.2 and a simple Gemfile?

PabloC commented Oct 25, 2011

and what about support for 1.9.2 and a simple Gemfile?

@seppo0010 seppo0010 referenced this pull request Oct 26, 2011

Closed

Cloud Foundry support #11

@guilleiguaran

This comment has been minimized.

Show comment
Hide comment
@guilleiguaran

guilleiguaran Oct 28, 2011

would be nice be able to specify RedisHost using ENV["REDIS_HOST"] env variable

would be nice be able to specify RedisHost using ENV["REDIS_HOST"] env variable

@PabloC

This comment has been minimized.

Show comment
Hide comment

PabloC commented on 61ebf8f Oct 28, 2011

+1!

@zhemao

This comment has been minimized.

Show comment
Hide comment
@zhemao

zhemao Oct 28, 2011

I've gotten rid of shotgun in the Gemfile and regenerated the Gemfile.lock. Changes are sitting at my fork https://github.com/zhemao/lamernews

zhemao commented Oct 28, 2011

I've gotten rid of shotgun in the Gemfile and regenerated the Gemfile.lock. Changes are sitting at my fork https://github.com/zhemao/lamernews

@antirez

This comment has been minimized.

Show comment
Hide comment
@antirez

antirez Oct 29, 2011

Owner

zhemao: thanks, but can't merge your as well as there are unrelated commits in your tree that are now fixed merging other pull requests.

Owner

antirez commented Oct 29, 2011

zhemao: thanks, but can't merge your as well as there are unrelated commits in your tree that are now fixed merging other pull requests.

@zhemao

This comment has been minimized.

Show comment
Hide comment
@zhemao

zhemao Oct 29, 2011

Blech, fixing merge conflicts would have been a nightmare, so I rolled back and added the changes back in manually. Hopefully it is acceptable now.

zhemao commented Oct 29, 2011

Blech, fixing merge conflicts would have been a nightmare, so I rolled back and added the changes back in manually. Hopefully it is acceptable now.

@fcambus

This comment has been minimized.

Show comment
Hide comment
@fcambus

fcambus Aug 11, 2013

Collaborator

We now have a Gemfile, introduced by this commit : 1fe117a

Closing.

Collaborator

fcambus commented Aug 11, 2013

We now have a Gemfile, introduced by this commit : 1fe117a

Closing.

@fcambus fcambus closed this Aug 11, 2013

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