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

Test against current and next Ruby and Rails versions #335

Merged
merged 19 commits into from
Nov 17, 2021

Conversation

ivy
Copy link
Collaborator

@ivy ivy commented Nov 17, 2021

Hey folks! This pull request sets up continuous integration (CI) through GitHub Actions and tests against the following Ruby and Rails releases (see a successful build here):

  • MRI: HEAD, 3.1.0-preview1, 3.0, 2.7, 2.6
  • JRuby: HEAD, 9.3, 9.2
  • TruffleRuby: HEAD, 21.3
  • Rails: Edge, 6.1, 6.0, 5.2

The versions listed above are actively maintained while other releases are beyond their end-of-life (EOL) are removed from the CI matrix.

Summary of changes

Dropping older Ruby and Rails versions

This may need some discussion but mainly, my rationale is that it would be better to point anyone dependent on older releases to older versions of the gem and continue forward with what's being supported by core maintainers. There's some interesting discussion in rspec/rspec-metagem#25 exploring how best to handle this kind of change and for a pre-1.0 project like this one, I think this is reasonable.

Switching from Travis CI to GitHub Actions

In retrospect, it might have been a good idea to ask about this beforehand! 😅 I like that the Ruby setup for GitHub Actions comes directly from the Ruby team (see ruby/setup-ruby) and it's nice to see everything in one UI. Otherwise, there isn't much of a difference. 🤷‍♀️

lograge.gemspec

The following changes are made to lograge.gemspec:

  • Ruby >= 2.5 is required. I felt pretty conflicted about this but I would prefer to move forward to what's currently supported.
  • Multifactor is required for administrative actions on Rubygems. Security ftw! 🙌
  • Development dependencies:
    • rspec is pinned to ~> 3.1 (from >= 0)
    • rubocop is pinned to ~> 1.23 (from 0.46.0)
    • simplecov is added with version ~> 0.21

Rubocop ~> 1.23

These are the most notable changes that came as part of the upgrade to rubocop ~> 1.23:

  • 6fee37c: All new cops are enabled and some small tweaks are made to reduce the amount of immediate violations
  • cd803be: Gemfiles are renamed from Gemfile.* to *.gemfile
  • f8269a0: Most violations are fixed with rubocop --auto-correct-all (code coverage suggests this was done without error)
  • a7bf717: Remaining violations are fixed by hand
  • 2eeb433: Formatters::Graylog2#call is changed to use Hash#transform_keys instead of cloning and modifying with .each.keys

Future work

This was really fun to work on! I think it sets things up to a good spot where I can dive a little deeper. Some closing thoughts (mostly a reminder to myself):

  • I'd like to drop in rubocop-performance and see if it can find and fix any low-hanging fruit. Off the top of my head, I know it will warn about the use of define_method in SilentLogger.
  • What the heck is going on in logstash-event? It's such a strange dependency and I don't like that it's pinned to an old tag. I'm curious to see if we can rip that dependency out and serialize events without it.

ivy added 19 commits November 17, 2021 00:33
Fixes the following error:

    NameError:
      uninitialized constant Lograge::SimpleDelegator

        class SilentLogger < SimpleDelegator
                             ^^^^^^^^^^^^^^^
This tweaks mostly work around the existing tests by disabling a
couple cops.
Fixes an issue with the ruby/setup-ruby@v1 GitHub action where Rubocop
fails while attempting to search the vendor/ directory.
The *.gemfile file extension appears to be well understood by rubocop
and likely other tools.
These fixes were automatically made with:

    rubocop --auto-correct-all
Rubocop auto-corrected `data_clone.each.keys` to `data_clone.each_key`
which resulted in the following error:

     RuntimeError:
       can't add a new key into hash during iteration

To fix this, we use Hash#transform_keys (introduced in Ruby 2.5.0).
@ivy ivy requested review from roidrage and benlovell November 17, 2021 09:01
Copy link
Collaborator

@benlovell benlovell left a comment

Choose a reason for hiding this comment

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

Awesome! Definitely happy about the move to GitHub actions as well. Your comment on logstash-event is spot on too. 🚀

@ivy ivy merged commit 2ab8482 into roidrage:master Nov 17, 2021
@ivy ivy deleted the ivy/github-actions-ci branch November 17, 2021 18:03
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