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

Remove linter errors (in .rubocop_todo.yml ) #77

Open
curquiza opened this issue Oct 12, 2021 · 8 comments
Open

Remove linter errors (in .rubocop_todo.yml ) #77

curquiza opened this issue Oct 12, 2021 · 8 comments
Labels
good first issue Good for newcomers

Comments

@curquiza
Copy link
Member

In .rubocop_todo.yml we currently have some remain offenses, we need to remove them :)
@curquiza curquiza mentioned this issue Oct 12, 2021
Closed
@hosamaly hosamaly mentioned this issue Oct 15, 2021
Merged
bors bot added a commit that referenced this issue Oct 16, 2021
@bors @hosamaly
Merge #78
f124e66 
78: Fix some rubocop warnings in tests r=curquiza a=hosamaly

In response to  #77, this merge request fixes many RuboCop issues in `integration_spec` and `utilities_spec`.

Most of them were auto-corrected, but care had to be taken because the auto-corrections sometimes resulted in tests that do nothing (e.g. changing `x.should == nil` to `x.nil?`).

Co-authored-by: Hosam Aly <hosamaly6@gmail.com>
@AudTheCodeWitch
Copy link

I would like to work on this issue, if possible.

@curquiza
Copy link
Member Author

Hello @AudTheCodeWitch

Thanks for your interest in this project :)

FYI, we prefer not assigning people to our issues because sometimes people ask to be assigned and never come back, which discourages the volunteer contributors to open a PR to fix this issue.
We will accept and merge the first PR that fixes correctly and well implements the issue following our contributing guidelines.

We are looking forward to reviewing your PR 🙂

@reginaalyssa
Copy link

Hi @curquiza I'm interested to work on this. Hope I can get some advice on a failing spec while running on local for spec/configuration_spec.rb. Below is what I ran:

  1. Ran ./meilisearch in one terminal
  2. While checked out on the main branch of meilisearch-rails, I ran bundle exec rspec spec/configuration_spec.rb
  3. I got the errors shown below:

image

Please let me know where I went wrong in running the steps. I unfortunately do not have docker installed on my laptop. Thank you so much!

@curquiza
Copy link
Member Author

Hello @reginaalyssa
Did you run the required commands you can find in the CONTRIBUTING.md before running the spec?

brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 1, 2021
@brunoocasali
Fix all the offenses on pagination files
f980629 
- TrailingWhitespace
- UnusedMethodArgument
- AndOr
- IfUnlessModifier
- StringLiterals

Related to meilisearch#77
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 1, 2021
@brunoocasali
Fix offenses from rake tasks
f6d24f0 
- HashSyntax
- TrailingWhitespace
- StringLiterals

Related to meilisearch#77
This was referenced Nov 1, 2021
Merged
Merged
bors bot added a commit that referenced this issue Nov 2, 2021
@bors @brunoocasali
Merge #81
cccf2d5 
81: Enhancement/Code style v3 r=curquiza a=brunoocasali

- Include .idea to gitignore file
- Fix offenses on spec_helper
- Fix offenses on pagination files
- Fix offenses on rake tasks
- Publish changes on rubocop_todo.yml


Related to #77 

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
bors bot added a commit that referenced this issue Nov 2, 2021
@bors @brunoocasali
Merge #82
86e5593 
82: Enhancement/Code style v4 r=curquiza a=brunoocasali

- Multiple fixes on specs
- Change configuration on rubocop to be more concise to the kind of meili tests.

Do not merge before #81

Related with #77 

After this PR and 77: `18 files inspected, 338 offenses detected, 160 offenses auto-correctable`

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 2, 2021
@brunoocasali
Fix remaining rubocop offenses on Gemfile
4f0d881 
Related to meilisearch#77
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 2, 2021
@brunoocasali
Fix multiple rubocop offenses on meilisearch-rails
dbc5cac 
Layout/EmptyLinesAroundAccessModifier
Layout/AssignmentIndentation
Layout/EmptyLineAfterGuardClause
Layout/ExtraSpacing
Layout/SpaceAfterComma
Layout/SpaceAroundOperators
Layout/Layout/SpaceBeforeComma
Layout/TrailingEmptyLines
Style/ConditionalAssignment
Style/GlobalStdStream
Style/MutableConstant
Style/NumericPredicate
Style/Proc
Style/RedundantBegin

Persist configuration to rubocop_todo.yml

Related to meilisearch#77
@brunoocasali brunoocasali mentioned this issue Nov 2, 2021
Merged
bors bot added a commit that referenced this issue Nov 2, 2021
@bors @brunoocasali
Merge #83
78b9b0e 
83: Enhancement/Code Style v5 r=curquiza a=brunoocasali

`@curquiza` sorry to flood your notification system 🤣 but the idea is to reduce the size of the diff to make it easier to review. 

Multiple offenses fixed like:

Layout/EmptyLinesAroundAccessModifier
Layout/AssignmentIndentation
Layout/EmptyLineAfterGuardClause
Layout/ExtraSpacing
Layout/SpaceAfterComma
Layout/SpaceAroundOperators
Layout/Layout/SpaceBeforeComma
Layout/TrailingEmptyLines
Style/ConditionalAssignment
Style/GlobalStdStream
Style/MutableConstant
Style/NumericPredicate
Style/Proc
Style/RedundantBegin

Related to #77 

before:
`18 files inspected, 365 offenses detected, 175 offenses auto-correctable`

after:
`18 files inspected, 288 offenses detected, 104 offenses auto-correctable`


Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 2, 2021
@brunoocasali
Simplify the pagination backend constant finding
0171649 
Related to meilisearch#77
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 2, 2021
@brunoocasali
Fix offenses on railtie/version files
1ba0c81 
Related to meilisearch#77
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 2, 2021
@brunoocasali
Fix multiple rubocop offenses on meilisearch-rails file
323876e 
Layout/EmptyLineBetweenDefs
Layout/EmptyLinesAroundBlockBody
Layout/IndentationConsistency
Lint/IneffectiveAccessModifier
Lint/RedundantStringCoercion
Lint/UselessAccessModifier
Naming/PredicateName
Rails/Blank
Rails/Present
Style/AndOr
Style/ClassEqualityComparison
Style/HashSyntax
Style/IdenticalConditionalBranches
Style/MutableConstant
Style/Next

Related to meilisearch#77
@brunoocasali brunoocasali mentioned this issue Nov 2, 2021
Merged
@brunoocasali
Copy link
Member

brunoocasali commented Nov 2, 2021 •
edited

@curquiza I was sending many PR's but I didn’t mention what I want with them, the current approach is to:

  • Fix small problems like double quote strings and others (Code/Style ## PR's)
  • Fix most of the auto-correctable offenses (or all) (Code/Style ## PR's)
  • Replace should with expect inside specs and offenses without changes on what the tests do.
  • Start decomposing the big methods inside of the meilisearch-rails in order to fix the cyclomatic complexity
  • Make tests independent (add randomization)

(feel free to reply with your concerns)

Probably after this, the rubocop_todo would be gone!

@curquiza
Copy link
Member Author

curquiza commented Nov 2, 2021

Hello @brunoocasali
thanks for these suggestions! 😄

We should create separated issues for what you suggest (the 3rd last points) since it's not related to this issue "Remove linter errors". Feel free to open them to detail what you plan to do a little bit 🙂
One issue per idea is ok.

bors bot added a commit that referenced this issue Nov 2, 2021
@bors @brunoocasali
Merge #84
d956076 
84: Enhancement/Code Style v6 r=curquiza a=brunoocasali

Fixed more offenses on meilisearch-rails, version, pagination files.

Layout/EmptyLineBetweenDefs
Layout/EmptyLinesAroundBlockBody
Layout/IndentationConsistency
Lint/IneffectiveAccessModifier
Lint/RedundantStringCoercion
Lint/UselessAccessModifier
Naming/PredicateName
Rails/Blank
Rails/Present
Style/AndOr
Style/ClassEqualityComparison
Style/HashSyntax
Style/IdenticalConditionalBranches
Style/MutableConstant
Style/Next
Style/ParenthesesAroundCondition
Style/PerlBackrefs 
Style/RedundantAssignment 
Style/RedundantParentheses 
Style/RedundantReturn

before:
`18 files inspected, 288 offenses detected, 104 offenses auto-correctable`

after:
`18 files inspected, 238 offenses detected, 58 offenses auto-correctable`


Related to #77 

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 2, 2021
@brunoocasali
Fix multiple rubocop errors on meilisearch-rails
24b7235 
Style/NegatedIfElseCondition
Style/Not
Style/NegatedIf
Style/HashConversion
Rails/Output
Layout/SpaceInsideArrayLiteralBrackets
Layout/MultilineOperationIndentation
Layout/EndAlignment
Layout/ElseAlignment

Related to meilisearch#77
@brunoocasali brunoocasali mentioned this issue Nov 2, 2021
Merged
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 2, 2021
@brunoocasali
Fix leaking constants definition on utilities file
e1db7ea 
Related to meilisearch#77
@brunoocasali brunoocasali mentioned this issue Nov 2, 2021
Merged
@brunoocasali
Copy link
Member

Yes! I'll provide the issues ASAP :)

@curquiza
Copy link
Member Author

curquiza commented Nov 3, 2021

Thanks a lot

bors bot added a commit that referenced this issue Nov 3, 2021
@bors @brunoocasali
Merge #86
9542213 
86: Code/Style v7 r=curquiza a=brunoocasali

Fix multiple rubocop issues again!

Style/NegatedIfElseCondition
Style/Not
Style/NegatedIf
Style/HashConversion
Rails/Output
Layout/SpaceInsideArrayLiteralBrackets
Layout/MultilineOperationIndentation
Layout/EndAlignment
Layout/ElseAlignment

Related to #77

after: `18 files inspected, 190 offenses detected, 16 offenses auto-correctable`

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
brunoocasali added a commit to brunoocasali/meilisearch-rails that referenced this issue Nov 3, 2021
@brunoocasali
Fix leaking constants definition on utilities file
cb3ad75 
Related to meilisearch#77
bors bot added a commit that referenced this issue Nov 3, 2021
@bors @brunoocasali
Merge #87
0757113 
87: Enhancement/Code Style v8 r=curquiza a=brunoocasali

Fix leaking constants on utilities tests.

Merge after #86 
Related to #77 

Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@brunoocasali @reginaalyssa @curquiza @AudTheCodeWitch @meili-bot