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

Enable RSpec related Rubocop rules #9227

Merged
merged 6 commits into from May 5, 2022
Merged

Enable RSpec related Rubocop rules #9227

merged 6 commits into from May 5, 2022

Conversation

alecslupu
Copy link
Contributor

@alecslupu alecslupu commented May 4, 2022 •
edited

🎩 What? Why?

Fixing

  • RSpec/IdenticalEqualityAssertion
  • RSpec/ExcessiveDocstringSpacing
  • RSpec/FactoryBot/SyntaxMethods
  • RSpec/StubbedMock

📌 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@alecslupu alecslupu added dependencies Pull requests that update a dependency file or issues that talk about updating dependencies type: internal PRs that aren't necessary to add to the CHANGELOG for implementers labels May 4, 2022
@alecslupu alecslupu force-pushed the ale-fix-rspec-rubocops branch 3 times, most recently from 2c60e82 to 5133ff3 Compare May 4, 2022 13:54
@alecslupu alecslupu marked this pull request as ready for review May 4, 2022 15:04
@alecslupu alecslupu requested a review from ahukkanen May 4, 2022 15:04
@ahukkanen ahukkanen changed the title Fix RSpec related rubocops Enable RSpec related Rubocop rules May 4, 2022
ahukkanen
ahukkanen requested changes May 4, 2022
View reviewed changes
Copy link
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

Few issues below I noticed if you could take a look please?

Also, did you leave RSpec/VerifiedDoubleReference disabled on purpose?

decidim/.rubocop-disabled.yml

Lines 124 to 125 in 5133ff3

RSpec/VerifiedDoubleReference:
Enabled: false

decidim-admin/spec/commands/decidim/admin/destroy_component_spec.rb Outdated Show resolved Hide resolved
decidim-sortitions/spec/services/decidim/sortitions/admin/draw_spec.rb Outdated Show resolved Hide resolved
decidim-core/spec/validators/geocoding_validator_spec.rb Outdated Show resolved Hide resolved
decidim-core/spec/validators/passthru_validator_spec.rb Outdated Show resolved Hide resolved
decidim-core/spec/validators/uploader_content_type_validator_spec.rb Outdated Show resolved Hide resolved
@alecslupu
Copy link
Contributor Author

Few issues below I noticed if you could take a look please?

Also, did you leave RSpec/VerifiedDoubleReference disabled on purpose?

decidim/.rubocop-disabled.yml

Lines 124 to 125 in 5133ff3

RSpec/VerifiedDoubleReference:
Enabled: false

Yes, that one is left commented on purpose, as there are some issues that i am planning to tackle later...

@alecslupu
Copy link
Contributor Author

I will have a look on the other items commented.

ahukkanen
ahukkanen previously approved these changes May 4, 2022
View reviewed changes
@ahukkanen
Copy link
Contributor

@alecslupu It looks good now but there is a conflict at .rubocop-disabled.yml.

Could you resolve that please?

@alecslupu alecslupu requested a review from ahukkanen May 5, 2022 06:47
@ahukkanen ahukkanen merged commit f686fa2 into decidim:develop May 5, 2022
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahukkanen ahukkanen ahukkanen approved these changes

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file or issues that talk about updating dependencies type: internal PRs that aren't necessary to add to the CHANGELOG for implementers
Projects
None yet
Milestone
0.27.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alecslupu @ahukkanen