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.

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

fix rubocop complains, activate rubocop again, add .rubocop_todo.yml #10

Conversation

SimonHoenscheid
Copy link
Sponsor Member

This Pull Request (PR) fixes the following issues

Fixes #9

There are 16 failed unit tests now. These are expecting stubbing now. Need to figure that out. Any hints welcome

@ananace
Copy link
Member

ananace commented Feb 24, 2023

receive and have_received are entirely different things; expect(Class).to receive(:method) would add a stub method onto a class, while expect(Class).to have_received(:method) only adds a spy on if an existing method has been called.

This is also seen in the messages on the allow(Class).to ... tests, since using have_received there is a no-op. (You're basically just saying "I don't mind if a method is called", which doesn't make much sense in a testing context.)

@SimonHoenscheid
Copy link
Sponsor Member Author

@ananace interesting. The Rubocop messages left me with the impression these were interchangeable. I will have a closer look.

@SimonHoenscheid
Copy link
Sponsor Member Author

I decided to exclude these checks. Pipeline is now OK.

@SimonHoenscheid SimonHoenscheid marked this pull request as ready for review March 1, 2023 07:29
Copy link
Member

@bastelfreak bastelfreak left a comment

Choose a reason for hiding this comment

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

looks okay to me, but we don't have acceptance tests to verify it's still working. It would be nice if someone else can take a look at well.

Copy link
Member

@ananace ananace left a comment

Choose a reason for hiding this comment

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

I personally dislike the period at the end of line change, but not going to hold things back on styling disagreements, especially against rubocop - which I assume is what prompted the change.

@SimonHoenscheid SimonHoenscheid merged commit 0caa18a into voxpupuli:master Mar 1, 2023
@SimonHoenscheid SimonHoenscheid deleted the shoenscheid_rubocop_complains branch March 1, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update ruby code to meet rubocops criterias
3 participants