briannesbitt / Carbon

A simple PHP API extension for DateTime.
https://carbon.nesbot.com/
MIT License
16.58k stars 1.28k forks source link

Fix incorrect function calls in Units #3088

Closed tben closed 1 month ago

tben commented 1 month ago

The CarbonInterval make function appears to accept the following arguments: $intervals, $units, and $skipCopy, but Units class is instead sending $units, an empty array, and true. This causes issues with the following calls ->sub('day') ->add('day') or ->subtract('day') on the Carbon object

Useful Link: https://github.com/briannesbitt/Carbon/blob/5607f63713142842b3cac9c9a69b371bc27f213b/src/Carbon/CarbonInterval.php#L1201C16-L1201C88

kylekatarnls commented 1 month ago

Hello,

What issue does it causes? As you can see with the unit tests, it breaks badly multiple features. I suggest you first open an issue with a description of the bug you're facing, how to reproduce and what behavior you were expecting.

We also expect merge request to come with a unit test showing what problem they are solving and then ensuring it would not be broken by a future change.

tben commented 1 month ago

Thanks for the quick response. I think I've misunderstand how this function works. My problem was that I'm trying to do Carbon::parse('now')->sub('day'); which in my mind would take away a day but just result in an exception. I was debugging it on my local vm and swapping these variables fixed it but I clearly didn't account for when ->sub('1 day') is used. Closing pull request since this isn't a good enough solution.