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

Adopt rubocop and format the project #2241

Merged
merged 9 commits into from
Feb 8, 2024
Merged

Adopt rubocop and format the project #2241

merged 9 commits into from
Feb 8, 2024

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Feb 4, 2024

In addition to having a consistent coding style through CI checks, anyone uses a LSP server (whether Ruby LSP or Solargraph) will have auto-formatting in their editor as well.

  1. After checking different rubocop rules, I think rubocop-rails-omakase's rules match my taste the most but also doesn't require much changes in our codebase.
    • I didn't use standard as it's not supported by Ruby LSP atm
  2. I ran rubocop -a on the entire project to apply the styles, with a couple of manual adjustments.
  3. I also added a new CI job for linting.

#skip-changelog

steps:
- uses: actions/checkout@v3
- name: Set up Ruby
uses: ruby/setup-ruby@v1
Copy link

Choose a reason for hiding this comment

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

  • 🚫 Please pin the action by specifying a commit SHA instead of a tag/branch.

Copy link

codecov bot commented Feb 4, 2024

Codecov Report

Merging #2241 (c534ee5) into master (3377a28) will increase coverage by 0.02%.
The diff coverage is 93.33%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2241      +/-   ##
==========================================
+ Coverage   97.41%   97.43%   +0.02%     
==========================================
  Files         102      102              
  Lines        3823     3823              
==========================================
+ Hits         3724     3725       +1     
+ Misses         99       98       -1     
Components Coverage Δ
sentry-ruby 98.10% <100.00%> (ø)
sentry-rails 95.05% <50.00%> (ø)
sentry-sidekiq 94.70% <100.00%> (ø)
sentry-resque 93.65% <ø> (+1.58%) ⬆️
sentry-delayed_job 95.60% <ø> (ø)
sentry-opentelemetry 100.00% <100.00%> (ø)
Files Coverage Δ
...entry-delayed_job/lib/sentry/delayed_job/plugin.rb 100.00% <ø> (ø)
...entelemetry/lib/sentry/opentelemetry/propagator.rb 100.00% <100.00%> (ø)
...lemetry/lib/sentry/opentelemetry/span_processor.rb 100.00% <100.00%> (ø)
sentry-rails/app/jobs/sentry/send_event_job.rb 60.00% <ø> (ø)
sentry-rails/lib/sentry/rails/configuration.rb 97.14% <ø> (ø)
...ls/lib/sentry/rails/tracing/abstract_subscriber.rb 74.07% <ø> (ø)
.../sentry/rails/tracing/active_storage_subscriber.rb 100.00% <ø> (ø)
sentry-ruby/lib/sentry-ruby.rb 96.44% <ø> (ø)
sentry-ruby/lib/sentry/check_in_event.rb 100.00% <100.00%> (ø)
sentry-ruby/lib/sentry/configuration.rb 98.78% <100.00%> (ø)
... and 17 more

... and 1 file with indirect coverage changes

@st0012 st0012 requested a review from sl0thentr0py February 4, 2024 14:44
@st0012 st0012 marked this pull request as ready for review February 4, 2024 14:44
rubocop-rails-omakase: rubocop.yml

Layout/SpaceInsideArrayLiteralBrackets:
Enabled: false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In addition to not liking this style myself, having it enabled requires changing almost all array literals in the codebase (~500 more offences). So I decided to not take this one in.

@st0012
Copy link
Collaborator Author

st0012 commented Feb 8, 2024

@sl0thentr0py 👋 let me know if you have concern about this?

Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

nice, thx!

@st0012 st0012 merged commit 2979e86 into master Feb 8, 2024
120 of 126 checks passed
@st0012 st0012 deleted the adopt-rubocop branch February 8, 2024 22:45
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