makandra / makandra-rubocop

makandra's default Rubocop configuration
MIT License
6 stars 1 forks source link

Discussion: Style/AccessorGrouping #27

Closed FLeinzi closed 3 years ago

FLeinzi commented 3 years ago

makandra-rubocop 6.0.0 activated the cop Style/AccessorGrouping. There are two different styles possible:

EnforcedStyle: grouped (default)

class Foo
  attr_reader :bar, :baz
end

EnforcedStyle: separated

class Foo
  attr_reader :bar
  attr_reader :baz
end

Which should be our default?

:+1: for grouped :-1: for separated :confused: : for disable Cop

codener commented 3 years ago

I think I have never used the separated style. Leaning towards keeping cop active with "grouped" style.

triskweline commented 3 years ago

I prefer the separated style to affect less code (in Git) when I add or remove an accessor. The grouped style makes a larger diff, has a higher probability of merge conflicts and blames all accessors to my commit.

That said I wouldn't reject a PR if someone used the grouped style, so I voted to disable.

denzelem commented 3 years ago

Just some additional notes, when some project considers to not disable the cop. It might make sense to think about other grouping usages for consistency, too. For example a controversial implementation of Rails validations, where its not only about the git diff / history.

class User < ActiveRecord::Base
  validate :email, :first_name, :last_name, :age, presence: true
  validate :age, numericality: true
end
class User < ActiveRecord::Base
  validate :email, presence: true
  validate :first_name, presence: true
  validate :last_name, presence: true
  validate :age, presence: true, numericality: true
end
triskweline commented 3 years ago

I prefer to separate my validations by attribute for the same reasons outlined above.

Also to keep a single ("rules for the age attribute") as close together in the code as feasible.