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 upper bound dependency limits from gemspec #711

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

rafaelfranca
Copy link
Collaborator

All that was needed to support Rails 7 in this gem was to update the
gemspec. This means that even in the most likely release to break
anything, a major release, we didn't needed to worry about breaking this
gem.

This upper bound is just creating unnecessary work for the
maintainers of this gem, which would need to release it without any code
change, and the users, which need to open issues asking for new releases.

Let's allow our users to try any version of our dependencies and report
real issues if they find them, and save everyone's time just removing
the upper bound.

@rafaelfranca rafaelfranca force-pushed the drop-upper-bound branch 3 times, most recently from 9a8d34b to 981d576 Compare January 19, 2022 20:32
@rafaelfranca rafaelfranca changed the title Remove upper bound limit for out dependencies Remove upper bound limit from our dependencies Jan 19, 2022
Copy link
Member

@javierjulio javierjulio left a comment

Choose a reason for hiding this comment

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

Thanks @rafaelfranca! I understand the reasoning and am ok with either removing or specifying upper bounds. Why did you remove the gemspec_lint test? That would help us confirm no warnings. Other than thought no concerns with the change. There are merge conflicts.

@rafaelfranca
Copy link
Collaborator Author

Why did you remove the gemspec_lint test? That would help us confirm no warnings

I removed because RubyGems show warnings for dependencies without upper bound. I wish I could just disable that warning so we could checking the others, but that is not the case

@deivid-rodriguez
Copy link
Member

I agree with your reasoning too @rafaelfranca. I would like to work on making RubyGems gemspec warnings configurable in the future. But for now it seems easy enough to ignore this particular warning from the output, instead of removing everything? I'm happy to work on it myself if you don't have the time.

@rafaelfranca
Copy link
Collaborator Author

You mean in the test? I can totally do it. I'll update the PR to ignore just that warning.

@deivid-rodriguez
Copy link
Member

Yes, in the test I mean.

@javierjulio javierjulio mentioned this pull request Oct 6, 2023
All that was needed to support Rails 7 in this gem was to update the
gemspec. This means that even in the most likely release to break
anything, a major release, we didn't needed to worry about breaking this
gem.

This upper bound is just creating unnecessary work for the
maintainers of this gem, which would need to release it without any code
change, and the users, which need to open issues asking for new releases.

Let's allow our users to try any version of our dependencies and report
real issues if they find them, and save everyone's time just removing
the upper bound.
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (c0f38ff) 98.19% compared to head (cdd983b) 98.19%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #711   +/-   ##
=======================================
  Coverage   98.19%   98.19%           
=======================================
  Files          14       14           
  Lines         555      555           
=======================================
  Hits          545      545           
  Misses         10       10           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@javierjulio javierjulio changed the title Remove upper bound limit from our dependencies Remove upper bound limits from gemspec Oct 6, 2023
@javierjulio javierjulio changed the title Remove upper bound limits from gemspec Remove upper bound dependency limits from gemspec Oct 6, 2023
Copy link
Member

@javierjulio javierjulio left a comment

Choose a reason for hiding this comment

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

Thanks!

@javierjulio javierjulio merged commit 204e34a into master Oct 6, 2023
22 checks passed
@javierjulio javierjulio deleted the drop-upper-bound branch October 6, 2023 22:27
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.

None yet

3 participants