Skip to content

Commit

Permalink
Remove pry from Gemfile
Browse files Browse the repository at this point in the history 
As far as I can tell, we dont need this on the Gemfile, as people can
just `gem install pry` and run:
```
pry -r ./config/environment
```
If they want to use pry.
  • Loading branch information
@arthurnn
arthurnn committed Mar 31, 2015
1 parent 8b3bac9 commit 20073cb
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 13 deletions.
4 changes: 0 additions & 4 deletions Gemfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ group :test do
gem 'timecop'
end

group :development, :test do
gem 'pry'
end

group :recovery do
gem "fakeredis"
end
Expand Down
8 changes: 0 additions & 8 deletions Gemfile.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ GEM
bcrypt
email_validator (~> 1.4)
rails (>= 3.1)
coderay (1.1.0)
coffee-rails (4.0.1)
coffee-script (>= 2.2.0)
railties (>= 4.0.0, < 5.0)
Expand Down Expand Up @@ -123,7 +122,6 @@ GEM
nokogiri (>= 1.5.9)
mail (2.6.3)
mime-types (>= 1.16, < 3)
method_source (0.8.2)
mime-types (2.4.3)
mini_portile (0.6.2)
minitest (5.5.1)
Expand All @@ -145,10 +143,6 @@ GEM
paul_revere (1.3)
rails (>= 3.0)
pg (0.18.1)
pry (0.10.1)
coderay (~> 1.1.0)
method_source (~> 0.8.1)
slop (~> 3.4)
psych (2.0.13)
rack (1.6.0)
rack-protection (1.5.3)
Expand Down Expand Up @@ -214,7 +208,6 @@ GEM
rack (~> 1.4)
rack-protection (~> 1.4)
tilt (~> 1.3, >= 1.3.4)
slop (3.6.0)
sprockets (2.12.3)
hike (~> 1.2)
multi_json (~> 1.0)
Expand Down Expand Up @@ -282,7 +275,6 @@ DEPENDENCIES
newrelic_rpm
paul_revere
pg
pry
psych (~> 2.0.12)
rack
rack-test
Expand Down
2 changes: 1 addition & 1 deletion vendor/cache
Submodule cache updated 4 files
+ − coderay-1.1.0.gem
+ − method_source-0.8.2.gem
+ − pry-0.10.1.gem
+ − slop-3.6.0.gem

6 comments on commit 20073cb

@arthurnn
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @sferik
(8664225)

@dwradcliffe
Copy link
Member

Choose a reason for hiding this comment

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

Who doesn't want to use pry?

@arthurnn
Copy link
Member Author

Choose a reason for hiding this comment

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

I dont know. I know that only adding pry to the Gemfile, the default rails console will not be pry , so i would assume people are not using it, as we dont default to it.
As I assumed nobody was using, and this cut about 100ms on boottime, I thought it was worth removing it.

before:

[arthurnn@ralph rubygems.org]$ time bundle exec rake environment
Digest::Digest is deprecated; use Digest

real    0m3.114s
user    0m2.463s
sys 0m0.647s

after:

[arthurnn@ralph rubygems.org]$ time bundle exec rake environment
Digest::Digest is deprecated; use Digest

real    0m2.933s
user    0m2.346s
sys 0m0.582s

If my assumptions were wrong, let me know so I will revert this.

@sferik
Copy link
Member

@sferik sferik commented on 20073cb Mar 31, 2015

Choose a reason for hiding this comment

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

I’m okay with this change but I’m not convinced by your run-once benchmark. The 100 milliseconds difference could easily be your system system busy with some other task or just random noise.

It’s true, pry was not the default console, but it was occasionally useful to run bundle exec pry to debug something. That said, I can always add it back to the Gemfile when I occasionally want to do that.

@arthurnn
Copy link
Member Author

Choose a reason for hiding this comment

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

The 100 milliseconds difference could easily be your system system busy with some other task or just random noise.

Thats right. The benchmark is really brittle.

I dont know.. TBH I dont care much, if we wanna add it back, it is fine.

@sferik
Copy link
Member

@sferik sferik commented on 20073cb Mar 31, 2015

Choose a reason for hiding this comment

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

Let’s leave it out for now and see how painful that is.

Please sign in to comment.