Skip to content

Add sum of expectation #20

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

Merged
merged 19 commits into from
Oct 8, 2024
Merged

Add sum of expectation #20

merged 19 commits into from
Oct 8, 2024

Conversation

Katalam
Copy link
Contributor

@Katalam Katalam commented Jul 29, 2024

When merge this will:

  • add .idea folder to git ignore
  • add a strict check to prime expectation
  • add a sum of expectation

@faissaloux
Copy link
Owner

Hello @Katalam

When I'll use toBeSumOf(), I'll expect to sum up the first argument like the other methods toBePowerOf() and toBeFactorialOf() ...

So to be consistent and not confusing, can you move the callable $step to the first position and make it able to accept an array as well, so it either sum up the array of numbers or the callable with $from to $to (Rename those params too please).

Thank you

@faissaloux
Copy link
Owner

I'm thinking about separate this to two methods:

  • toBeSumOf() should accept only an array of numbers.
  • Another one that accepts $step, $from and $to (This order should be respected) .

@faissaloux faissaloux self-assigned this Aug 5, 2024
@faissaloux
Copy link
Owner

ping @Katalam

@Katalam
Copy link
Contributor Author

Katalam commented Oct 4, 2024

I broke my forked git tree somehow, but I removed all the unwanted commit changes

@faissaloux faissaloux merged commit 20c1f5d into faissaloux:main Oct 8, 2024
22 checks passed
@faissaloux
Copy link
Owner

@Katalam thanks for your contribution!

@Katalam Katalam deleted the sum-of branch October 8, 2024 12:55
@faissaloux faissaloux added this to the 1.4.0 milestone Oct 9, 2024
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