kpumuk commented on code in PR #3716:
URL: https://github.com/apache/thrift/pull/3716#discussion_r3787976430
##########
lib/rb/spec/client_spec.rb:
##########
@@ -91,18 +91,18 @@ def close
expect(trans).to receive(:flush)
end
end
- klass = double("TestMessage_args", :new => mock_args)
- @client.send_message("testMessage", klass, :foo => "foo", :bar => 42)
+ klass = double("TestMessage_args", new: mock_args)
+ @client.send_message("testMessage", klass, foo: "foo", bar: 42)
end
Review Comment:
Addressed. This call now passes `{foo: "foo", bar: 42}` explicitly as the
third positional argument.
##########
compiler/cpp/src/thrift/generate/t_rb_generator.cc:
##########
@@ -961,7 +961,7 @@ void t_rb_generator::generate_service_client(t_service*
tservice) {
f_service_.indent() << messageSendProc << "(\"" << funname << "\", " <<
argsname;
for (fld_iter = fields.begin(); fld_iter != fields.end(); ++fld_iter) {
- f_service_ << ", :" << (*fld_iter)->get_name() << " => " <<
(*fld_iter)->get_name();
+ f_service_ << ", " << (*fld_iter)->get_name() << ": " <<
(*fld_iter)->get_name();
}
Review Comment:
Addressed. The generator now surrounds service fields with `{...}`, and the
compiler functional test `t_rb_generator emits service arguments as a
positional hash` asserts the exact generated call.
##########
lib/rb/spec/struct_spec.rb:
##########
@@ -68,7 +68,7 @@ def validate; end
def validate_default_arguments(object)
expect(object.simple).to eq(53)
expect(object.words).to eq("words")
- expect(object.hello).to eq(SpecNamespace::Hello.new(:greeting => "hello,
world!"))
+ expect(object.hello).to eq(SpecNamespace::Hello.new(greeting: "hello,
world!"))
Review Comment:
Not applicable. `Thrift::Struct#initialize(d = {}, &block)` declares no
keyword parameters, so Ruby passes this label-style argument list as its single
positional Hash. The CI matrix passes these exact specs on Ruby 2.7, every Ruby
3.x release, Ruby 4.0, and Ruby head.
##########
lib/rb/spec/thin_http_server_spec.rb:
##########
@@ -82,36 +82,36 @@
path = "/thin"
expect(Thin::Server).to receive(:new).with(ip, port,
an_instance_of(Rack::Builder))
Thrift::ThinHTTPServer.new(processor,
- :ip => ip,
- :port => port,
- :path => path)
+ ip: ip,
+ port: port,
+ path: path)
end
Review Comment:
Addressed. These `ThinHTTPServer` constructor calls now pass their options
as explicit positional Hash literals.
##########
lib/rb/spec/http_client_spec.rb:
##########
@@ -224,7 +224,7 @@
it "should set SSL verify mode when specified" do
client =
Thrift::HTTPClientTransport.new("#{@server_uri}#{@service_path}",
- :ssl_verify_mode => OpenSSL::SSL::VERIFY_NONE)
+ ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE)
Review Comment:
Addressed. The `HTTPClientTransport` calls now pass `ssl_verify_mode` and
`ssl_ca_file` in explicit positional Hash literals.
##########
test/rb/generation/test_struct.rb:
##########
@@ -38,7 +38,7 @@ def test_default_values
assert_kind_of(Hash, hello.complex)
assert_equal(hello.complex, { 6243 => 632, 2355 => 532, 23 => 532})
- bool_passer = TestNamespace::BoolPasser.new(:value => false)
+ bool_passer = TestNamespace::BoolPasser.new(value: false)
Review Comment:
Not applicable. `Thrift::Struct#initialize(d = {}, &block)` declares no
keyword parameters, so Ruby passes this label-style argument list as its single
positional Hash. The CI matrix passes these exact specs on Ruby 2.7, every Ruby
3.x release, Ruby 4.0, and Ruby head.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
