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

Introduce withNumTests and deprecate withMaxSuccess #365

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaximilianAlgehed
Copy link
Collaborator

After a discussion about changing the interaction between checkCoverage and withMaxSuccess whereby checkCoverage will ignore the number given in withMaxSuccess (because checkCoverage may require more tests than what is specified) it was decided that a more intuitive name for withMaxSuccess is withNumTests. The new name reflects the fact that withNumTests does not set a hard limit (unlike what is indicated by the Max in withMaxSuccess) on the number of tests that are run.

@MaximilianAlgehed
Copy link
Collaborator Author

I tried to get a grip on the number of people we will annoy with this change by searching for the following query on github withMaxSuccess language:haskell NOT is:archived NOT is:fork and it gave back ~600 files (with at most 50 files being instances of withMaxSuccess used in a comment).

Now, the thing I wasn't able to ascertain was how many distinct repositories would be impacted, but would I suspect it's somewhere between 60 and 300.

@MaximilianAlgehed
Copy link
Collaborator Author

The reason for making this change is the change in #383 that means that maxSuccess is never really max success (it wasn't before either - but now it's even more so not the case). Weighing that against pissing up to 600 people off however doesn't immediately worth while to me.

My plan is to wait with this change until we have enough other changes that will annoy people that we can't ignore and just bundle all the breaking changes in one place.

@nick8325
Copy link
Owner

Another option would be to introduce withNumTests but not yet mark withMaxSuccess as {-# DEPRECATED #-}. We could still update the Haddocks to tell people to use withNumTests instead.

@phadej
Copy link
Contributor

phadej commented Mar 26, 2024

@MaximilianAlgehed https://hackage-search.serokell.io/ is better corpus to search through

@MaximilianAlgehed
Copy link
Collaborator Author

@MaximilianAlgehed https://hackage-search.serokell.io/ is better corpus to search through

Thank you! I'll do a more thorough search there.

@MaximilianAlgehed
Copy link
Collaborator Author

There are about 50 odd packages on hackage that use withMaxSuccess that we have to care about (i.e. aren't mentioning it only in a comment).

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.

3 participants