-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix: Format gat commands #1312
base: main
Are you sure you want to change the base?
fix: Format gat commands #1312
Conversation
97264dd
to
1e84858
Compare
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.
Looks gat to me!
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.
Looks like gat
isn't supported in (one of?) the dalli
version we test against. We should consider whether to bump the minimum version we support.
@@ -127,6 +127,14 @@ | |||
_(span.name).must_equal 'set' | |||
_(span.attributes['db.statement']).must_equal 'set ?' | |||
end | |||
|
|||
it 'supports gat' do | |||
dalli.gat('foo') |
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.
gat
was added to dalli
in 3.0.0. We could skip
if the dalli version is < 3.0.0 or set 3.0.0 as the min supported version.
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.
My preference is to skip
, since we only need it in this test, and the test matrix should cover 3.0.0.
We don't know how to format gat, lets help the instrumentation do it properly.