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

[feat]: Allow wait to accept 0, false and nil to disable waiting. #40

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

manuelmeurer
Copy link
Contributor

Resolves #39


Before the change?

Only :exponentially or a positive number was accepted as the wait parameter.

After the change?

0, false and nil are accepted as the wait parameter to disable waiting.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@manuelmeurer manuelmeurer requested a review from a team as a code owner September 28, 2023 10:13
@manuelmeurer
Copy link
Contributor Author

Hmm, not sure why all the other commits we included in this PR, let me try to fix that...

@manuelmeurer
Copy link
Contributor Author

I changed the other tests that were using wait: 0.01 to use wait: false, otherwise it would be confusing why they are using a tiny delay when wait: false is also possible.

@manuelmeurer
Copy link
Contributor Author

Actually, we don't need the check for duration in the test I added, since the timeout param would raise an error if there was any wait... shall we remove it?

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

This is looking good. As far as the tests go, it would be nice to have a few of the specs still implement numerical values as a check to ensure there were no regressions or breaking change with the implementation there.

Basically a check to ensure that non zero and above waits still perform as intended (negatives seem to be accounted for in the tests currently)

@nickfloyd nickfloyd added the Type: Feature New feature or request label Sep 28, 2023
@nickfloyd nickfloyd changed the title No wait [feat]: Allow wait to accept 0, false and nil to disable waiting. Sep 28, 2023
@manuelmeurer
Copy link
Contributor Author

Sure, makes sense, I changed four occurrences back to wait: 0.01.

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Thank you again for these commits! ❤️

@nickfloyd nickfloyd merged commit 9de9443 into octokit:main Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FEAT]: No wait
2 participants