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.

Sign up for GitHub

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

Jump to bottom

Update rubocop to latest and fix as many violations as possible #29

Merged
merged 16 commits into from
May 14, 2024
Merged

Update rubocop to latest and fix as many violations as possible #29

merged 16 commits into from
May 14, 2024

Conversation

lstrzebinczyk
Copy link
Contributor

@lstrzebinczyk lstrzebinczyk commented May 13, 2024 •
edited
Loading

As in the title. We were able to fix about 1000 violations in automated manner, I manually fixed a bunch others, but hundreds of violations remain. I created a todo file to cut the work at where it's right now.

granite-form codebase is in a poor shape.

AC:

  • Trust specs

@lstrzebinczyk lstrzebinczyk requested a review from a team as a code owner May 13, 2024 09:25
@lstrzebinczyk lstrzebinczyk self-assigned this May 13, 2024
@lstrzebinczyk
Copy link
Contributor Author

@toptal-anvil ping reviewers

@kml kml requested a review from a team May 13, 2024 09:38
sasha370
sasha370 approved these changes May 13, 2024
View reviewed changes
Copy link

@sasha370 sasha370 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

mbie
mbie approved these changes May 13, 2024
View reviewed changes
Copy link

@mbie mbie left a comment

Choose a reason for hiding this comment

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

Please check the suggestion

spec/granite/form/types/date_time_spec.rb Show resolved Hide resolved
spec/granite/form/types/time_spec.rb Show resolved Hide resolved
id-ilych
id-ilych reviewed May 13, 2024
View reviewed changes
Copy link
Contributor

@id-ilych id-ilych left a comment

Choose a reason for hiding this comment

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

Autofix did a mostly good job, but some human touch could be nice

lib/granite/form/model/associations/nested_attributes.rb Outdated Show resolved Hide resolved
spec/granite/form/active_record/associations_spec.rb Outdated Show resolved Hide resolved
spec/granite/form/active_record/associations_spec.rb Outdated Show resolved Hide resolved
spec/granite/form/model/associations/embeds_many_spec.rb Outdated Show resolved Hide resolved
spec/granite/form/model/associations/embeds_many_spec.rb Outdated Show resolved Hide resolved
spec/support/shared/nested_attribute_examples.rb Outdated Show resolved Hide resolved
spec/support/shared/nested_attribute_examples.rb Outdated Show resolved Hide resolved
spec/support/shared/nested_attribute_examples.rb Outdated Show resolved Hide resolved
spec/support/shared/nested_attribute_examples.rb Outdated Show resolved Hide resolved
lib/granite/form/model/associations/base.rb Outdated Show resolved Hide resolved
@lstrzebinczyk lstrzebinczyk force-pushed the devx-4047-bump-rubocop branch 2 times, most recently from 5bc554c to 353a7fe Compare May 13, 2024 11:26
lstrzebinczyk and others added 15 commits May 13, 2024 14:31
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/associations/embeds_many_spec.rb
c31f20a 
Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/associations/embeds_one_spec.rb
567d2f6 
Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/associations/reflections/embeds_many_s…
cac186d 
…pec.rb

Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/associations/reflections/embeds_many_s…
93801de 
…pec.rb

Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/associations/reflections/references_ma…
5bf1016 
…ny_spec.rb

Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/attributes/attribute_spec.rb
6772c66 
Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/attributes/attribute_spec.rb
35a009c 
Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/attributes/attribute_spec.rb
335cf88 
Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk @id-ilych
Update spec/granite/form/model/attributes/base_spec.rb
79e6fca 
Co-authored-by: Ilya Denisov <ilya.denisov@toptal.com>
@lstrzebinczyk
wip
25c7901
@lstrzebinczyk
Merge branch 'devx-4047-bump-rubocop' of github.com:toptal/granite-fo…
f07dff6 
…rm into devx-4047-bump-rubocop
@lstrzebinczyk
wip
0256948
@lstrzebinczyk
wip
84db185
@lstrzebinczyk
wip
ea68770
@lstrzebinczyk
wip
9e95f69
@lstrzebinczyk lstrzebinczyk merged commit 8b96bf4 into master May 14, 2024
5 checks passed
@lstrzebinczyk lstrzebinczyk deleted the devx-4047-bump-rubocop branch May 14, 2024 08:08
@catks catks mentioned this pull request May 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kml kml kml approved these changes

@mbie mbie mbie approved these changes

@id-ilych id-ilych id-ilych approved these changes

@sasha370 sasha370 sasha370 approved these changes

Assignees

@lstrzebinczyk lstrzebinczyk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lstrzebinczyk @kml @mbie @id-ilych @sasha370