GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Remove pry from Gemfile as it is not used by dvzrv · Pull Request #608 · ruby-i18n/i18n

Conversation

@dvzrv

@dvzrv dvzrv commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

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

Fixes #607

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@dvzrv
Remove pry from Gemfile as it is not used
bd48ee0 
Gemfile:
The pry gem is installed but does not seem to be used for anything.
Tests pass without it just fine.

Fixes #607
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Jan 27, 2022
@dvzrv svntogit
upgpkg: ruby-i18n 1.9.0-1: Upgrade to 1.9.0.
f2b296d 
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
@dvzrv
upgpkg: ruby-i18n 1.9.0-1: Upgrade to 1.9.0.
b2c4cd3 
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

radar commented Jan 27, 2022

Copy link
Copy Markdown
Collaborator

It's used when I debug stuff.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@radar radar closed this Jan 27, 2022
@dvzrv

dvzrv commented Jan 27, 2022

Copy link
Copy Markdown
Contributor Author

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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@radar

radar commented Jan 28, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] pry is not required during testing

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@dvzrv @radar
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.