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

CI-1852: Update to a more modern ruby version #36

Merged
merged 3 commits into from
Feb 24, 2023

Conversation

DragosDumitrache
Copy link

@DragosDumitrache DragosDumitrache commented Feb 23, 2023

Moved to using Ruby 3.1.3 and updated our gem dependencies

Prometheus client had a breaking API change so we have to add named
arguments properly now

Had to use named arguments for counters as well as histograms
Also updated rubocop to use cops appropriate to the ruby version being used

For context on named arguments:
prometheus/client_ruby@160c25e

Moved to using Ruby 3.1.3 and updated our gem dependencies
Committed the Gemfile.lock as it's typically good practice to ensure
replicable builds

Prometheus client had a breaking API change so we have to add named
arguments properly now

Had to use named arguments for counters as well as histograms
Also updated rubocop to use cops appropriate to the ruby version being used
@DragosDumitrache DragosDumitrache force-pushed the CI-1852-update-ruby branch 2 times, most recently from 87affd1 to 31feec4 Compare February 23, 2023 11:13
@EsmeAR EsmeAR requested a review from a team February 23, 2023 11:16
@@ -0,0 +1,31 @@
name: Test
Copy link
Author

Choose a reason for hiding this comment

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

Copied from upstream, only change is adding our main branch to be gocardless

Choose a reason for hiding this comment

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

this i'm assuming will be removed when we move the code back to upstream i guess?

Choose a reason for hiding this comment

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

(the gocardless bit i mean 🤣 )

@@ -1,43 +1,60 @@
---
# This configuration was generated by
Copy link
Author

Choose a reason for hiding this comment

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

Generated by rubocop, all standard

Comment on lines 58 to +61
:"#{metric_prefix}_messages_compression_duration_seconds",
"Time taken to compress a batch of messages",
{},
[0, 0.0001, 0.0005, 0.001, 0.01, 0.05, 0.1, 0.25, 0.5, 1],
docstring: "Time taken to compress a batch of messages",
buckets: [0, 0.0001, 0.0005, 0.001, 0.01, 0.05, 0.1, 0.25, 0.5, 1],
labels: [:topic, :algorithm]

Choose a reason for hiding this comment

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

these is because of a breaking change on their side i guess?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, a former GC person made this change but we've never updated this on our side

Copy link

@kevinrobayna kevinrobayna left a comment

Choose a reason for hiding this comment

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

LGTM! pretty simple PR though can you change the description of the PR as we are no longer adding the Gemfile.lock file

@DragosDumitrache DragosDumitrache merged commit 7f68bc4 into gocardless Feb 24, 2023
DragosDumitrache added a commit that referenced this pull request Feb 24, 2023
* Update to a more modern version of ruby

Moved to using Ruby 3.1.3 and updated our gem dependencies

Prometheus client had a breaking API change so we have to add named
arguments properly now

Had to use named arguments for counters as well as histograms
Also updated rubocop to use cops appropriate to the ruby version being used

* Add CI pipeline
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.

2 participants