-
Notifications
You must be signed in to change notification settings - Fork 225
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
Drop support for ruby 2.4.x #407
Conversation
Hey @etagwerker, Will you need anything from me to make this happen? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rishijain for your first contribution! I've allowed the tests to go through.
There's only a small consideration here, let me know what you think.
Hey @nunosilva800, Can you help me understand what broke this? Also, I saw similar failures in the last release #405. So this suggests that these failures may not be related to any change I did. What do you think? |
Dir.chdir(PathHelper.project_path) | ||
yield | ||
ensure | ||
FakeFS::FileSystem.clear | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing the begin
from the block since th target ruby version in rubocop.yml is set to 2.5 and from ruby 2.5 onwards begin block is seemed redundant as per rubocop rules.
here is that PR when the change was done in rubocop rubocop/rubocop#5175
@rishijain I'm still unsure on the exactly why builds for ruby 2.5 are failing. Since Ruby 2.5 is also no longer maintained, I think we can also remove it from the build steps. I'll merge this PR in and raise a new issue. Thanks! |
awesome. thanks, @nunosilva800 . |
2.6 is EOL, but if the CI is still happy with it, I'd say keep it in there. We just not bother with fixing things when the underlying infra / dependencies stop working. |
Fixes #406
Check list: