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

Drop support for Ruby 2.5 as its close to EOL #782

Merged
merged 4 commits into from
Jan 14, 2021

Conversation

karanthukral
Copy link
Contributor

What are you trying to accomplish with this PR?

  • Dropping support for ruby 2.5 in favour of switching the default to 2.6.6

How is this accomplished?

  • Update gemspec, CI and local dev

What could go wrong?

  • Unexpected issues with ruby upgrade but the version is being tested on CI

@karanthukral karanthukral requested a review from a team as a code owner January 13, 2021 18:24
@CautionTapeBot
Copy link

Shopify's style guide prefers allow-list/deny-list vs. white-list/black-list.

Copy link
Contributor

@dturn dturn left a comment

Choose a reason for hiding this comment

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

Just some minor stuff

command: bin/ci
agents:
queue: k8s-ci
env:
LOGGING_LEVEL: "4"
KUBERNETES_VERSION: v1.19-latest
RUBY_VERSION: "2.7"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 (Assuming the idea is to test each version of ruby w/o building an entire matrix)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup! 2.6 default and then 2.7 and 3 on single k8s versions

CHANGELOG.md Outdated Show resolved Hide resolved
dev.yml Outdated
@@ -1,7 +1,7 @@
---
name: krane
up:
- ruby: 2.5.7 # Matches gemspec
- ruby: 3.0.0 # Matches gemspec
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 2.6.6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes.. was using it for testing. Will set it to 2.6.6

kubectl.expects(:run).with { |*_args, **kwargs| kwargs[:output_is_sensitive] == true }.returns([
"Some Raw Output",
"Error from kubectl: something went wrong and by the way here's your secret: S3CR3T",
stub(success?: false),
])
kubectl.expects(:run)
Copy link
Contributor

Choose a reason for hiding this comment

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

That's strange that this changed just from the Ruby version :/ I would have that it specific to Mocha. Regardless, I think it's ok for now, though I'm not a huge fan of introducing so much static info into the stub :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Ruby 2.7 deprecated some stuff with keyword args I haven't fully read up on it https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ but maybe this smells like it could be related.

@karanthukral karanthukral merged commit 1d137a6 into master Jan 14, 2021
@karanthukral karanthukral deleted the karan/drop-ruby-2-5 branch January 14, 2021 19:39
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

4 participants