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

chore: remove ostruct and benchmark from gemspec #742

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

oehlschl
Copy link

@oehlschl oehlschl commented Feb 7, 2025

Fixes #743

Follow-up to #738 and #739.

From what I saw, ostruct was added to the gemspec because it was needed by a rubocop fromatter as part of the test-and-deploy.yml GitHub CI workflow, which failed on a deprecation warning becaue ostruct will removed from the Ruby std lib in v3.5.0 and therefore has to be installed explicitly.

Adding these dependencies to Twilio's gemspec does "solve" the problem by making the gems available, but it also indicates that they are required fo Twilio to function, which I don't think is true; I didn't see any references to OpenStruct in the Twilio source, and the gem was ultimately loaded by rubocop as part of a CI step (where rubcop is actually added to the bundle individually, here); thus, ostruct seems to be required by the Twilio CI workflow but not by Twilio itself. (And I assume the same is true for benchmark.)

This PR removes ostruct and benchmark from the Twilio gemspec, which both avoids the confusion that they are required for Twilio and also prevents future users of this gem from having to install those dependences, which they do today (though it's currently moot since ostruct is still part of the Ruby std lib, so they get it anyway). It keeps the gems defined in the Gemfile, which should keep them available to bundle-aware operations like the CI workflow.

Note that the real test of this will be re-running "Test (ruby-head)" which was where the errors were first seen.

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@oehlschl oehlschl changed the title remove ostruct and benchmark from gemspec chore: remove ostruct and benchmark from gemspec Feb 7, 2025
@oehlschl oehlschl marked this pull request as ready for review February 7, 2025 23:55
@oehlschl
Copy link
Author

oehlschl commented Feb 7, 2025

Thanks @tiwarishubham635!

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.

ostruct and benchmark should not be production Twilio dependencies
1 participant