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

Remove pry from Gemfile as it is not used #608

Merged
merged 1 commit into from Jan 28, 2022
Merged

Conversation

dvzrv
Copy link
Contributor

@dvzrv dvzrv commented Jan 27, 2022

Gemfile:
The pry gem is installed but does not seem to be used for anything.
Tests pass without it just fine.

Fixes #607

Gemfile:
The pry gem is installed but does not seem to be used for anything.
Tests pass without it just fine.

Fixes ruby-i18n#607
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Jan 27, 2022
Add upstream patch to remove broken imports:
ruby-i18n/i18n#602
Add upstream patch to remove unused pry gem:
ruby-i18n/i18n#608
Simplify quoting in file.
Use check() to run tests (finally).
Various style fixes and simplifications.

git-svn-id: file:///srv/repos/svn-community/svn@1117463 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Jan 27, 2022
Add upstream patch to remove broken imports:
ruby-i18n/i18n#602
Add upstream patch to remove unused pry gem:
ruby-i18n/i18n#608
Simplify quoting in file.
Use check() to run tests (finally).
Various style fixes and simplifications.

git-svn-id: file:///srv/repos/svn-community/svn@1117463 9fca08f4-af9d-4005-b8df-a31f2cc04f65
@radar
Copy link
Collaborator

radar commented Jan 27, 2022

It's used when I debug stuff.

@radar radar closed this Jan 27, 2022
@dvzrv
Copy link
Contributor Author

dvzrv commented Jan 27, 2022

It's used when I debug stuff.

I understand that, but having it in the general Gemfile, which is used e.g. by downstream distributions implies that it is used (and required) during testing.

Adding this package to the Gemfile means that downstream distributions (when not investigating whether pry is used in the remaining codebase or not) are required to package pry and its entire dependency tree (e.g. coderay and all of its dependencies).

This is significantly more work than just not adding pry to the Gemfile, especially given that you can install pry separately if I'm not mistaken.

From a distributor perspective: I would like to be able to run tests. If I have to package 10 more packages (that are actually not required) it is more likely that I will not run tests (which is rather to the detriment of i18n users in general and distribution users in particular).

Is there a way for you to install the pry gem on the side, so that it is not required in the Gemfile? Alternatively, can there be a comment in the Gemfile that clarifies that this gem is actually not required to run tests?

@radar
Copy link
Collaborator

radar commented Jan 28, 2022

You make a fine point. Let's remove it.

@radar radar reopened this Jan 28, 2022
@radar radar merged commit f33af4f into ruby-i18n:master Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] pry is not required during testing
2 participants