RuboCop Changes in Foreman - Input needed

Development
1

I’m currently working on addressing RuboCop fix requirements for Foreman by raising PRs for specific directories. Here’s an overview:

While fixing Layout/ArgumentAlignment, I noticed potentially widespread indentation changes. I’m unsure if introducing these changes across the entire codebase is desirable, given the potential impact. Additionally, there are likely further Layout-related fixes on the horizon. My question is:

  1. Should we apply Layout/ArgumentAlignment changes considering their potential impact on indentation?
  2. Also how we want to proceed with addressing Style/* and Lint/* cops as well?

I appreciate your insights and opinions on this matter.

1 Like
2

Could you give some examples of how the code would be changed?

3

In the past we had Michael who always submitted a PR with one single cop per PR. Then we had the debate on that PR if it made sense. Combining multiple cops in a single PR makes it harder to review and discuss the impact. If there were two styles, he would also open 2 PRs so you could easily see both changes and compare them.

A random example: Fixes #19770 - Fix Layout/DotPosition cop by mmoll · Pull Request #5419 · theforeman/foreman · GitHub and Fixes #19770 - Disable Layout/DotPosition cop by mmoll · Pull Request #5414 · theforeman/foreman · GitHub is how we historically dealt with these cops.

Of course, this predates theforeman-rubocop with its common rules. These days you may need to take plugins into account.

4

Sample PR: Fix Layout/ArgumentAlignment by archanaserver · Pull Request #10051 · theforeman/foreman · GitHub

I have created this draft PR to look for the code change