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

Binary Search spec: Use verifying doubles consistently where introduced #52

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

macauleydev
Copy link
Contributor

Because

  • This file, 15b_binary_search_spec.rb, as explained in its comments, "uses verifying doubles to help you get acqauintance with the concept". But it replaces only 2 of the file's 4 doubles with verifying doubles.
  • The inconsistency is not explained, so it creates unnecessary confusion.
  • For what it's worth, a related file, 15a_binary_game_spec, uses verifying doubles consistently.

This PR

  • Removes the unexplained inconsistency from 15b by replacing the remaining 2 doubles with verifying doubles.
  • (Also removes some trailing spaces. Not sure if Prettier or RuboCop deserves credit/blame. Hopefully not a problem.)

Issue

N/A

Additional Information

I found a Discord discussion where a student mentioned confusion about this inconsistency and @rlmoser99 invited a PR, but it looks like none was made.

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this PR follows the location of change: brief description of change format, e.g. String spec: Update instructions for clarity
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If this PR includes changes in the spec folder, they are also updated in the corresponding file in the spec_answers folder (with passing tests). [N/A, as this file is informational and has no corresponding answer file]

Copy link
Member

@rlmoser99 rlmoser99 left a comment

Choose a reason for hiding this comment

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

Thanks. I am not sure how those got out of sync, so thanks for making a PR.

@rlmoser99 rlmoser99 merged commit c80ee63 into TheOdinProject:main Dec 15, 2023
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.

2 participants