Skip to content

Conversation

@smusali
Copy link
Contributor

@smusali smusali commented Dec 16, 2019

The purpose of this Pull Request is to automate the release pipeline:

  • updated logdna.gemspec
  • replaced the old LICENSE.txt with the newer LICENSE
  • removed Rakefile and .rspec since there is no structured example or test suite
  • removed test.rb since it is integration testing without checking the end result
  • created CircleCI Config by creating the following pipeline in a tagged build:
  1. test: right now, it's running only RuboCop
  2. build: it creates gem file to be used
  3. release: it creates a new GitHub Release and uploads the gem file created in the second step
  4. approve: it is approval stage before release job
  5. publish: it pushes the gem file to RubyGems

@smusali smusali self-assigned this Dec 16, 2019
@smusali smusali marked this pull request as ready for review December 16, 2019 09:04
@smusali
Copy link
Contributor Author

smusali commented Dec 17, 2019

@vilyapilya, I see that you introduced/added RuboCop to this Code Library in #7 but now you are saying whatever RuboCop reports and autocorrects is not good or recommended despite the fact that RuboCop is a Ruby static code analyzer and formatter, based on the community Ruby style guide powered by Open Source Ruby Community which is day-by-day improving its functionality, as said here

Remove unnecessary use of cat.
Copy link
Contributor

@mindjiver mindjiver left a comment

Choose a reason for hiding this comment

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

From the CircleCI changes I'm pretty happy, looking as nice and clean as we can get without code sharing via an Orb. For the Ruby part I think there are people more suited with stronger/better opinions.

@smusali
Copy link
Contributor Author

smusali commented Dec 17, 2019

From the CircleCI changes I'm pretty happy, looking as nice and clean as we can get without code sharing via an Orb. For the Ruby part I think there are people more suited with stronger/better opinions.

Thanks, @mindjiver, we are gonna have a separate meeting for discussing this!

Copy link
Contributor

@dchai76 dchai76 left a comment

Choose a reason for hiding this comment

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

@smusali - as a general rule don't get hung up on Rubcop's suggestions. It's not like e.g. PEP8 for Python - there is no single standard for Ruby style and Rubocop will enforce a ton of things according to your preference - it can enforce opposite styles. That's why rubocop's yamls are typically massive - everybody customizes it. E.g. I like to enforce parens for all method calls; others prefer the opposite. There's no single "right" way in Ruby so let's go with what we feel reads best.

@smusali
Copy link
Contributor Author

smusali commented Dec 20, 2019

@smusali - as a general rule don't get hung up on Rubcop's suggestions. It's not like e.g. PEP8 for Python - there is no single standard for Ruby style and Rubocop will enforce a ton of things according to your preference - it can enforce opposite styles. That's why rubocop's yamls are typically massive - everybody customizes it. E.g. I like to enforce parens for all method calls; others prefer the opposite. There's no single "right" way in Ruby so let's go with what we feel reads best.

That's right! I will revert my codebase changes back - touching codebase isn't part of this Pull Request

@smusali
Copy link
Contributor Author

smusali commented Dec 20, 2019

@vilyapilya, reverted codebase changes back but removed .rspec, .ruby-version, and Rakefile, test.rb, and the following dependencies from logdna.gemspec: bundler, rake, rspec, webmock since there is currently no structured test until this Pull Request and I think these test-related files and dependencies should be added or updated in #11

And I used .rubocop.yml modified in #11 and it reports errors on codebase; we'll need to address those errors or RuboCop targets in a separate Pull Request

@smusali smusali removed the request for review from andkon December 20, 2019 19:16
@smusali
Copy link
Contributor Author

smusali commented Dec 20, 2019

@dchai76 @vilyapilya, this Pull Request is good to go now with the following report by RuboCop which is going to be handled in separate Pull Request or maybe #11 :

.rubocop.yml: Metrics/LineLength has the wrong namespace - should be Layout
Inspecting 6 files
CWCCCC

Offenses:

Gemfile:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
source 'https://rubygems.org'
^
Gemfile:1:8: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
source 'https://rubygems.org'
       ^^^^^^^^^^^^^^^^^^^^^^
Gemfile:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
source "https://rubygems.org"
^
lib/logdna.rb:53:5: W: Lint/DuplicateMethods: Method Logdna::Ruby#level= is defined at both lib/logdna.rb:18 and lib/logdna.rb:53.
    def level=(value)
    ^^^^^^^^^^
lib/logdna.rb:63:7: C: [Corrected] Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
      if message.nil? && block_given?
      ^^
lib/logdna.rb:79:25: C: [Corrected] Layout/FirstArgumentIndentation: Indent the first argument one step more than opts.merge(.
                        level: lvl
                        ^^^^^^^^^^
lib/logdna.rb:80:9: C: [Corrected] Style/RedundantSelf: Redundant self detected.
        self.log(msg, opts.merge( ...
        ^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna.rb:80:23: C: [Corrected] Layout/ClosingParenthesisIndentation: Indent ) to column 17 (not 22)
                      ), &block)
                      ^
lib/logdna.rb:86:38: C: [Corrected] Style/RedundantSelf: Redundant self detected.
        return Resources::LOG_LEVELS[self.level] == lvl if level.is_a? Numeric
                                     ^^^^^^^^^^
lib/logdna.rb:88:9: C: [Corrected] Style/RedundantSelf: Redundant self detected.
        self.level == lvl
        ^^^^^^^^^^
lib/logdna.rb:120:7: C: [Corrected] Style/NegatedIf: Favor unless over if for negative conditions.
      @client.exitout if !@client.nil?
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna.rb:120:7: C: [Corrected] Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method.
      @client.exitout if !@client.nil?
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna.rb:122:7: C: [Corrected] Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
      if !@client.nil?
      ^^
lib/logdna.rb:122:7: C: [Corrected] Style/NegatedIf: Favor unless over if for negative conditions.
      if !@client.nil? ...
      ^^^^^^^^^^^^^^^^
lib/logdna.rb:122:7: C: [Corrected] Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method.
      if !@client.nil? ...
      ^^^^^^^^^^^^^^^^
lib/logdna.rb:124:7: C: [Corrected] Style/NegatedIf: Favor unless over if for negative conditions.
      @client.exitout if !@client.nil?
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna.rb:124:7: C: [Corrected] Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method.
      @client.exitout if !@client.nil?
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna.rb:128:7: C: [Corrected] Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
      if !@client.nil?
      ^^
lib/logdna.rb:128:7: C: [Corrected] Style/NegatedIf: Favor unless over if for negative conditions.
      if !@client.nil? ...
      ^^^^^^^^^^^^^^^^
lib/logdna.rb:128:7: C: [Corrected] Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method.
      if !@client.nil? ...
      ^^^^^^^^^^^^^^^^
lib/logdna/client.rb:117:1: C: [Corrected] Layout/EmptyLinesAroundMethodBody: Extra empty line detected at method body beginning.
lib/logdna/client.rb:120:9: C: [Corrected] Style/IfUnlessModifier: Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
        if @buffer.any? || @side_messages.any?
        ^^
lib/logdna/resources.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
module Resources
^
lib/logdna/resources.rb:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
module Resources
^
lib/logdna/resources.rb:2:16: C: [Corrected] Style/WordArray: Use %w or %W for an array of words.
  LOG_LEVELS = ['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL', 'TRACE'].freeze
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna/resources.rb:2:17: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  LOG_LEVELS = ['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL', 'TRACE'].freeze
                ^^^^^^^
lib/logdna/resources.rb:2:26: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  LOG_LEVELS = ['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL', 'TRACE'].freeze
                         ^^^^^^
lib/logdna/resources.rb:2:34: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  LOG_LEVELS = ['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL', 'TRACE'].freeze
                                 ^^^^^^
lib/logdna/resources.rb:2:42: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  LOG_LEVELS = ['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL', 'TRACE'].freeze
                                         ^^^^^^^
lib/logdna/resources.rb:2:51: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  LOG_LEVELS = ['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL', 'TRACE'].freeze
                                                  ^^^^^^^
lib/logdna/resources.rb:2:60: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  LOG_LEVELS = ['DEBUG', 'INFO', 'WARN', 'ERROR', 'FATAL', 'TRACE'].freeze
                                                           ^^^^^^^
lib/logdna/resources.rb:3:16: C: [Corrected] Style/WordArray: Use %w or %W for an array of words.
  LOG_LEVELS = ["DEBUG", "INFO", "WARN", "ERROR", "FATAL", "TRACE"].freeze
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna/resources.rb:3:30: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  DEFAULT_REQUEST_HEADER = { 'Content-Type' => 'application/json; charset=UTF-8' }.freeze
                             ^^^^^^^^^^^^^^
lib/logdna/resources.rb:3:48: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  DEFAULT_REQUEST_HEADER = { 'Content-Type' => 'application/json; charset=UTF-8' }.freeze
                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna/resources.rb:4:29: C: [Corrected] Style/NumericLiterals: Use underscores(_) as thousands separator and separate every 3 digits with them.
  DEFAULT_REQUEST_TIMEOUT = 180000
                            ^^^^^^
lib/logdna/resources.rb:5:17: C: [Corrected] Style/NumericLiterals: Use underscores(_) as thousands separator and separate every 3 digits with them.
  MS_IN_A_DAY = 86400000
                ^^^^^^^^
lib/logdna/resources.rb:6:25: C: [Corrected] Style/NumericLiterals: Use underscores(_) as thousands separator and separate every 3 digits with them.
  MAX_REQUEST_TIMEOUT = 300000
                        ^^^^^^
lib/logdna/resources.rb:7:21: C: [Corrected] Style/NumericLiterals: Use underscores(_) as thousands separator and separate every 3 digits with them.
  MAX_LINE_LENGTH = 32000
                    ^^^^^
lib/logdna/resources.rb:11:22: C: [Corrected] Style/NumericLiterals: Use underscores(_) as thousands separator and separate every 3 digits with them.
  FLUSH_BYTE_LIMIT = 500000
                     ^^^^^^
lib/logdna/resources.rb:12:14: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  ENDPOINT = 'https://logs.logdna.com/logs/ingest'.freeze
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna/resources.rb:13:14: C: [Corrected] Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.
  ENDPOINT = "https://logs.logdna.com/logs/ingest".freeze
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna/resources.rb:13:20: C: [Corrected] Style/MutableConstant: Freeze mutable objects assigned to constants.
  MAC_ADDR_CHECK = /^([0-9a-fA-F][0-9a-fA-F]:){5}([0-9a-fA-F][0-9a-fA-F])$/
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna/resources.rb:14:19: C: [Corrected] Style/MutableConstant: Freeze mutable objects assigned to constants.
  IP_ADDR_CHECK = /^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/logdna/version.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
module LogDNA
^
lib/logdna/version.rb:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
module LogDNA
^
lib/logdna/version.rb:2:13: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  VERSION = '1.3.0'.freeze
            ^^^^^^^
lib/logdna/version.rb:3:13: C: [Corrected] Style/RedundantFreeze: Do not freeze immutable objects, as freezing them has no effect.
  VERSION = "1.3.0".freeze
            ^^^^^^^^^^^^^^
logdna.gemspec:1:1: C: [Corrected] Style/Encoding: Unnecessary utf-8 encoding comment.
# coding: utf-8
^^^^^^^^^^^^^^^
logdna.gemspec:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
# coding: utf-8
^
logdna.gemspec:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
lib = File.expand_path('../lib', __FILE__)
^
logdna.gemspec:2:1: C: [Corrected] Layout/LeadingEmptyLines: Unnecessary blank line at the beginning of the source.
lib = File.expand_path('lib', __dir__)
^^^
logdna.gemspec:2:12: C: [Corrected] Style/ExpandPathArguments: Use expand_path('lib', __dir__) instead of expand_path('../lib', __FILE__).
lib = File.expand_path('../lib', __FILE__)
           ^^^^^^^^^^^
logdna.gemspec:2:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
lib = File.expand_path('../lib', __FILE__)
                       ^^^^^^^^
logdna.gemspec:4:9: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
require 'logdna/version'
        ^^^^^^^^^^^^^^^^
logdna.gemspec:7:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.name          = 'logdna'
                       ^^^^^^^^
logdna.gemspec:9:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.authors       = 'Gun Woo Choi, Derek Zhou, Vilya Levitskiy, Muaz Siddiqui'
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
logdna.gemspec:10:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.email         = 'support@logdna.com'
                       ^^^^^^^^^^^^^^^^^^^^
logdna.gemspec:11:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.summary       = 'LogDNA Ruby logger'
                       ^^^^^^^^^^^^^^^^^^^^
logdna.gemspec:12:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.homepage      = 'https://github.com/logdna/ruby'
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
logdna.gemspec:13:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.license       = 'MIT'
                       ^^^^^
logdna.gemspec:14:52: C: [Corrected] Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ].
  spec.files         = Dir.glob("{lib}/**/*.rb") + %w(LICENSE README.md)
                                                   ^^^^^^^^^^^^^^^^^^^^^
logdna.gemspec:15:24: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.bindir        = 'exe'
                       ^^^^^
logdna.gemspec:17:25: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.require_paths = ['lib']
                        ^^^^^
logdna.gemspec:18:31: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'concurrent-ruby', '~> 1.0'
                              ^^^^^^^^^^^^^^^^^
logdna.gemspec:18:50: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'concurrent-ruby', '~> 1.0'
                                                 ^^^^^^^^
logdna.gemspec:19:31: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'require_all', '~> 1.4'
                              ^^^^^^^^^^^^^
logdna.gemspec:19:39: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'json', '~> 2.0'
                                      ^^^^^^^^
logdna.gemspec:19:46: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'require_all', '~> 1.4'
                                             ^^^^^^^^
logdna.gemspec:20:3: C: [Corrected] Gemspec/OrderedDependencies: Dependencies should be sorted in an alphabetical order within their section of the gemspec. Dependency json should appear before require_all.
  spec.add_runtime_dependency 'json', '~> 2.0'
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
logdna.gemspec:20:31: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'json', '~> 2.0'
                              ^^^^^^
logdna.gemspec:20:39: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'json', '~> 2.0'
                                      ^^^^^^^^
logdna.gemspec:20:46: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_runtime_dependency 'require_all', '~> 1.4'
                                             ^^^^^^^^
logdna.gemspec:21:35: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_development_dependency 'rubocop', '~> 0.78'
                                  ^^^^^^^^^
logdna.gemspec:21:46: C: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  spec.add_development_dependency 'rubocop', '~> 0.78'
                                             ^^^^^^^^^

6 files inspected, 74 offenses detected, 73 offenses corrected

Exited with code exit status 1

@smusali smusali requested a review from dchai76 December 20, 2019 19:28
Copy link
Contributor

@dchai76 dchai76 left a comment

Choose a reason for hiding this comment

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

I'll approve this but we need to followup with the Rubocop issues. Part of it is we're not specifying the version of Rubocop to use anywhere; I'm accustomed to listing that in the Gemfile but I've not dealt with public Ruby libs before so it may not fit in. Let's discuss the specific rules in #integrations.

@smusali smusali merged commit 58b0f90 into master Dec 20, 2019
@smusali smusali deleted the initial_release_pipeline branch December 20, 2019 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants