avro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From fo...@apache.org
Subject [avro] branch master updated: AVRO-2482: Introduce RuboCop for linting the Ruby bindings (#592)
Date Mon, 05 Aug 2019 08:07:24 GMT
This is an automated email from the ASF dual-hosted git repository.

fokko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/master by this push:
     new 9125e9a  AVRO-2482: Introduce RuboCop for linting the Ruby bindings (#592)
9125e9a is described below

commit 9125e9acff342accbb404270c09fef4ccff151d6
Author: Kengo Seki <sekikn@apache.org>
AuthorDate: Mon Aug 5 17:07:18 2019 +0900

    AVRO-2482: Introduce RuboCop for linting the Ruby bindings (#592)
    
    * AVRO-2482: Introduce RuboCop for linting the Ruby bindings
    
    * Additional fixes for AVRO-2482
    
    * Rename unused arguments to _original_name rather than _ for readability
    
    * Fix Dockerfile to consolidate definition of the dependent libraries
    
    * Remove a shebang line and execution permission from test/random_data.rb,
      which only contains a class definition, and instead add them to
      sample_ipc_http_server.rb, which works as a standalone script
    
    * Additional fixes for AVRO-2482
    
    * Upgrade Yetus with some workarounds so that installing
      Ruby packages via Gemfile in docker works on Travis CI
    
    * Fix Ruby test tools to work with the IPC interop test
---
 .travis/before_install.sh                |  8 +++++++-
 .travis/script.sh                        |  5 ++++-
 lang/ruby/Gemfile                        |  1 +
 lang/ruby/build.sh                       |  2 +-
 lang/ruby/interop/test_interop.rb        |  0
 lang/ruby/lib/avro.rb                    |  2 +-
 lang/ruby/lib/avro/io.rb                 | 10 +++++-----
 lang/ruby/lib/avro/ipc.rb                |  4 ++--
 lang/ruby/lib/avro/schema.rb             | 12 ++++++------
 lang/ruby/test/case_finder.rb            |  4 ++--
 lang/ruby/test/random_data.rb            |  1 -
 lang/ruby/test/sample_ipc_client.rb      |  0
 lang/ruby/test/sample_ipc_http_client.rb |  0
 lang/ruby/test/sample_ipc_http_server.rb |  0
 lang/ruby/test/sample_ipc_server.rb      |  0
 lang/ruby/test/test_schema.rb            |  2 +-
 lang/ruby/test/tool.rb                   |  6 +++---
 share/docker/Dockerfile                  |  4 +++-
 18 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/.travis/before_install.sh b/.travis/before_install.sh
index b091cc5..87982e2 100755
--- a/.travis/before_install.sh
+++ b/.travis/before_install.sh
@@ -23,7 +23,13 @@ case "$TRAVIS_OS_NAME" in
     sudo apt-get -q install --no-install-recommends -y curl git gnupg-agent locales pinentry-curses
pkg-config rsync software-properties-common
     sudo apt-get -q clean
     sudo rm -rf /var/lib/apt/lists/*
-    curl -L https://www-us.apache.org/dist/yetus/0.8.0/yetus-0.8.0-bin.tar.gz | tar xvz -C
/tmp/
+
+    # Only Yetus 0.9.0+ supports `ADD` and `COPY` commands in Dockerfile
+    curl -L https://www-us.apache.org/dist/yetus/0.10.0/apache-yetus-0.10.0-bin.tar.gz |
tar xvz -C /tmp/
+    # A dirty workaround to disable the Yetus robot for TravisCI,
+    # since it'll cancel the changes that .travis/script.sh will do,
+    # even if the `--dirty-workspace` option is specified.
+    rm /tmp/apache-yetus-0.10.0/lib/precommit/robots.d/travisci.sh
     ;;
 "windows")
     choco install dotnetcore-sdk --version 2.2.300
diff --git a/.travis/script.sh b/.travis/script.sh
index adacc3d..00f1610 100755
--- a/.travis/script.sh
+++ b/.travis/script.sh
@@ -20,7 +20,10 @@ set -e
 case "$TRAVIS_OS_NAME" in
 "linux")
     sed -i.bak "s/openjdk:8/openjdk:${JAVA}/" share/docker/Dockerfile
-    /tmp/yetus-0.8.0/bin/test-patch --plugins=buildtest --java-home=/usr/local/openjdk-"${JAVA}"
--user-plugins=share/precommit/ --run-tests --empty-patch --docker --dockerfile=share/docker/Dockerfile
--dirty-workspace --verbose=true
+    # Workaround for Yetus. For now, Yetus assumes the directory in which Dockerfile is placed
is the docker context.
+    # So the Dockerfile should be here to refer to other subdirectories than share/docker
from inside the Dockerfile.
+    cp share/docker/Dockerfile .
+    /tmp/apache-yetus-0.10.0/bin/test-patch --plugins=buildtest --java-home=/usr/local/openjdk-"${JAVA}"
--user-plugins=share/precommit/ --run-tests --empty-patch --docker --dockerfile=Dockerfile
--dirty-workspace
     ;;
 "windows")
     ./lang/csharp/build.sh test
diff --git a/lang/ruby/Gemfile b/lang/ruby/Gemfile
index 59872d3..bb5514f 100644
--- a/lang/ruby/Gemfile
+++ b/lang/ruby/Gemfile
@@ -20,3 +20,4 @@ gem 'multi_json'
 gem 'snappy'
 gem 'zstd-ruby'
 gem 'test-unit'
+gem 'rubocop'
diff --git a/lang/ruby/build.sh b/lang/ruby/build.sh
index 1f1319d..3cb81c2 100755
--- a/lang/ruby/build.sh
+++ b/lang/ruby/build.sh
@@ -30,7 +30,7 @@ bundle install
 
 case "$1" in
     lint)
-      echo 'This is a stub where someone can provide linting.'
+      rubocop --lint
       ;;
 
     test)
diff --git a/lang/ruby/interop/test_interop.rb b/lang/ruby/interop/test_interop.rb
old mode 100644
new mode 100755
diff --git a/lang/ruby/lib/avro.rb b/lang/ruby/lib/avro.rb
index 38db245..8fca1ed 100644
--- a/lang/ruby/lib/avro.rb
+++ b/lang/ruby/lib/avro.rb
@@ -28,7 +28,7 @@ module Avro
 
   class AvroTypeError < Avro::AvroError
     def initialize(schm=nil, datum=nil, msg=nil)
-      msg ||= "Not a #{schm.to_s}: #{datum}"
+      msg ||= "Not a #{schm}: #{datum}"
       super(msg)
     end
   end
diff --git a/lang/ruby/lib/avro/io.rb b/lang/ruby/lib/avro/io.rb
index 04f08ba..9a1863e 100644
--- a/lang/ruby/lib/avro/io.rb
+++ b/lang/ruby/lib/avro/io.rb
@@ -172,7 +172,7 @@ module Avro
       end
 
       # null is written as zero bytes
-      def write_null(datum)
+      def write_null(_datum)
         nil
       end
 
@@ -292,7 +292,7 @@ module Avro
         readers_schema.type_adapter.decode(datum)
       end
 
-      def read_fixed(writers_schema, readers_schema, decoder)
+      def read_fixed(writers_schema, _readers_schema, decoder)
         decoder.read(writers_schema.size)
       end
 
@@ -359,7 +359,7 @@ module Avro
         readers_fields_hash = readers_schema.fields_hash
         read_record = {}
         writers_schema.fields.each do |field|
-          if readers_field = readers_fields_hash[field.name]
+          if (readers_field = readers_fields_hash[field.name])
             field_val = read_data(field.type, readers_field.type, decoder)
             read_record[field.name] = field_val
           else
@@ -468,7 +468,7 @@ module Avro
         decoder.skip(writers_schema.size)
       end
 
-      def skip_enum(writers_schema, decoder)
+      def skip_enum(_writers_schema, decoder)
         decoder.skip_int
       end
 
@@ -545,7 +545,7 @@ module Avro
         end
       end
 
-      def write_fixed(writers_schema, datum, encoder)
+      def write_fixed(_writers_schema, datum, encoder)
         encoder.write(datum)
       end
 
diff --git a/lang/ruby/lib/avro/ipc.rb b/lang/ruby/lib/avro/ipc.rb
index 97dbe02..26f6abd 100644
--- a/lang/ruby/lib/avro/ipc.rb
+++ b/lang/ruby/lib/avro/ipc.rb
@@ -278,7 +278,7 @@ module Avro::IPC
           response = call(local_message, request)
         rescue AvroRemoteError => e
           error = e
-        rescue Exception => e
+        rescue Exception => e # rubocop:disable Lint/RescueException
           error = AvroRemoteError.new(e.to_s)
         end
 
@@ -350,7 +350,7 @@ module Avro::IPC
       remote_protocol
     end
 
-    def call(local_message, request)
+    def call(_local_message, _request)
       # Actual work done by server: cf. handler in thrift.
       raise NotImplementedError
     end
diff --git a/lang/ruby/lib/avro/schema.rb b/lang/ruby/lib/avro/schema.rb
index c270e94..75937b9 100644
--- a/lang/ruby/lib/avro/schema.rb
+++ b/lang/ruby/lib/avro/schema.rb
@@ -143,11 +143,11 @@ module Avro
       SchemaCompatibility.mutual_read?(other_schema, self)
     end
 
-    def ==(other, seen=nil)
+    def ==(other, _seen=nil)
       other.is_a?(Schema) && type_sym == other.type_sym
     end
 
-    def hash(seen=nil)
+    def hash(_seen=nil)
       type_sym.hash
     end
 
@@ -165,7 +165,7 @@ module Avro
       end
     end
 
-    def to_avro(names=nil)
+    def to_avro(_names=nil)
       props = {'type' => type}
       props['logicalType'] = logical_type if logical_type
       props
@@ -182,7 +182,7 @@ module Avro
         super(type, logical_type)
         @name, @namespace = Name.extract_namespace(name, namespace)
         @doc  = doc
-        names = Name.add_name(names, self)
+        Name.add_name(names, self)
       end
 
       def to_avro(names=Set.new)
@@ -206,7 +206,7 @@ module Avro
 
       def self.make_field_objects(field_data, names, namespace=nil)
         field_objects, field_names = [], Set.new
-        field_data.each_with_index do |field, i|
+        field_data.each do |field|
           if field.respond_to?(:[]) # TODO(jmhodges) wtffffff
             type = field['type']
             name = field['name']
@@ -324,7 +324,7 @@ module Avro
         @symbols = symbols
       end
 
-      def to_avro(names=Set.new)
+      def to_avro(_names=Set.new)
         avro = super
         avro.is_a?(Hash) ? avro.merge('symbols' => symbols) : avro
       end
diff --git a/lang/ruby/test/case_finder.rb b/lang/ruby/test/case_finder.rb
index 2886417..3855127 100644
--- a/lang/ruby/test/case_finder.rb
+++ b/lang/ruby/test/case_finder.rb
@@ -44,7 +44,7 @@ class CaseFinder
   private
 
   def scan_case
-    if id = @scanner.scan(/\/\/ \d+\n/)
+    if (id = @scanner.scan(/\/\/ \d+\n/))
       while @scanner.skip(/\/\/ .*\n/); end
 
       input = scan_input
@@ -61,7 +61,7 @@ class CaseFinder
   def scan_item(name)
     if @scanner.scan(/<<#{name}\n/)
       lines = []
-      while line = @scanner.scan(/.+\n/)
+      while (line = @scanner.scan(/.+\n/))
         break if line.chomp == name
         lines << line
       end
diff --git a/lang/ruby/test/random_data.rb b/lang/ruby/test/random_data.rb
index 14cda15..bb4bb56 100644
--- a/lang/ruby/test/random_data.rb
+++ b/lang/ruby/test/random_data.rb
@@ -1,4 +1,3 @@
-#!/usr/bin/env ruby
 # Licensed to the Apache Software Foundation (ASF) under one
 # or more contributor license agreements.  See the NOTICE file
 # distributed with this work for additional information
diff --git a/lang/ruby/test/sample_ipc_client.rb b/lang/ruby/test/sample_ipc_client.rb
old mode 100644
new mode 100755
diff --git a/lang/ruby/test/sample_ipc_http_client.rb b/lang/ruby/test/sample_ipc_http_client.rb
old mode 100644
new mode 100755
diff --git a/lang/ruby/test/sample_ipc_http_server.rb b/lang/ruby/test/sample_ipc_http_server.rb
old mode 100644
new mode 100755
diff --git a/lang/ruby/test/sample_ipc_server.rb b/lang/ruby/test/sample_ipc_server.rb
old mode 100644
new mode 100755
diff --git a/lang/ruby/test/test_schema.rb b/lang/ruby/test/test_schema.rb
index 6fbc9cf..144fc81 100644
--- a/lang/ruby/test/test_schema.rb
+++ b/lang/ruby/test/test_schema.rb
@@ -455,5 +455,5 @@ class TestSchema < Test::Unit::TestCase
     end
     assert_equal('Error validating default for veggies: at . expected type null, got string
with value "apple"',
                  exception.to_s)
-    end
+  end
 end
diff --git a/lang/ruby/test/tool.rb b/lang/ruby/test/tool.rb
index b3072bb..e0a1246 100644
--- a/lang/ruby/test/tool.rb
+++ b/lang/ruby/test/tool.rb
@@ -27,7 +27,7 @@ class GenericResponder < Avro::IPC::Responder
     @datum = datum
   end
 
-  def call(message, request)
+  def call(message, _request)
     if message.name == @msg
       STDERR.puts "Message: #{message.name} Datum: #{@datum.inspect}"
       @datum
@@ -101,7 +101,7 @@ def main
       case ARGV[4]
       when "-file"
         Avro::DataFile.open(ARGV[5]) {|f|
-          f.each{|d| datum = d; break }
+          f.each{|e| datum = e; break }
         }
       when "-data"
         puts "JSON Decoder not yet implemented."
@@ -124,7 +124,7 @@ def main
     if ARGV.size > 4
       case ARGV[4]
       when "-file"
-        Avro::DataFile.open(ARGV[5]){|f| f.each{|d| datum = d; break } }
+        Avro::DataFile.open(ARGV[5]){|f| f.each{|e| datum = e; break } }
       when "-data"
         puts "JSON Decoder not yet implemented"
         return 1
diff --git a/share/docker/Dockerfile b/share/docker/Dockerfile
index ab59d3b..7dc9504 100644
--- a/share/docker/Dockerfile
+++ b/share/docker/Dockerfile
@@ -45,6 +45,7 @@ RUN apt-get -qq update && \
     asciidoc \
     bison \
     bzip2 \
+    bundler \
     cmake \
     curl \
     dotnet-sdk-2.2 \
@@ -106,7 +107,8 @@ RUN pip install zstandard
 RUN pip3 install zstandard
 
 # Install Ruby modules
-RUN gem install echoe yajl-ruby multi_json snappy zstd-ruby
+COPY lang/ruby/Gemfile /tmp
+RUN bundle install --gemfile=/tmp/Gemfile
 
 # Install global Node modules
 RUN npm install -g grunt-cli


Mime
View raw message