flipp-oss / deimos

Framework to work with Kafka, Avro and ActiveRecord
Other
59 stars 22 forks source link

GUILD-618: Add bundle install to lint step #157

Closed angad-singh closed 1 year ago

angad-singh commented 2 years ago

Pull Request Template

Description

This PR fixes the lint step in the Github CI. It was failing because the rubocop gem was not available in the CI. I fixed it by running the bundle install command with the option --with development to install the development dependencies.

The last 3 commits fix all the rubocop errors throughout the codebase. I used the autocorrect option while running rubocop and added rubocop:disable blocks for other errors.

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

N/A

Checklist:

dorner commented 2 years ago

@angad-singh the tests may start working if you update to the latest avro_turf (>= 1). You can do this in the Gemfile (it won't affect the gemspec).

angad-singh commented 2 years ago

@angad-singh the tests may start working if you update to the latest avro_turf (>= 1). You can do this in the Gemfile (it won't affect the gemspec).

@dorner Did you mean the files in .gemfiles like this? https://github.com/flipp-oss/deimos/blob/master/.gemfiles/avro_turf-0.gemfile

I changed it in this file https://github.com/flipp-oss/deimos/blob/master/deimos-ruby.gemspec which I am not so sure is the right way. (The tests didn't run due to an error in bundle install)

dorner commented 2 years ago

No, it's not, because the gemspec affects all downstream users of the library. I meant the actual Gemfile in the base directory.

angad-singh commented 2 years ago

@dorner that didn't work :( Unless I did something wrong

dorner commented 2 years ago

Ack I forgot that the gemfiles actually are installing avro_turf :D Yes you'd need to put it in those .gemfiles files. My bad!

dorner commented 2 years ago

But I guess the whole point is that things have to work with both versions. So we'd need to figure out why the tests are failing at all.