-
Notifications
You must be signed in to change notification settings - Fork 38
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
Refactors before 1.7 release #555
Conversation
383583c
to
c573bb3
Compare
57e669d
to
4e5e0bb
Compare
4e5e0bb
to
cd144e9
Compare
describe Documentation do | ||
subject = Documentation.new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only now noticed that the rule name is kinda off:
Ameba::Rule::Documentation::Documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I always feel uncomfortable with these massive refactorings across the codebase. In most big projects, contributions that involve refactoring are often discouraged because many suggestions tend to be subjective and, in some cases, introduce hidden issues.
I’m okay with this one, however, wanna add that refactoring just for the sake of cleaner code can sometimes create unnecessary churn and increasing the risk of introducing subtle bugs, especially before the release.
@veelenga I understand your sentiment, and in general I'm against such big PRs, yet once in a while I tend to gather all smaller (and thus with no or very slight chances of regressions) changes into one batch to save us time and energy that would go into reviewing these independently. |
No description provided.