Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Clock] Providemodify() in MockClock#48189
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedNov 10, 2022
Hey! Thanks for your PR. You are targeting branch "6.2" but it seems your PR description refers to branch "6.3". Cheers! Carsonbot |
dd49d4b to449c90dComparedbrumann commentedNov 10, 2022
I figured it's a bit late to get this into 6.2, which is why I targeted 6.3 even though the branch is not there yet. I know there were intense discussions about what should be part of the Clock in the original PR already, so feel free to close this if this functionality was omitted intentionally. |
449c90d to314d08bCompareUh oh!
There was an error while loading.Please reload this page.
javiereguiluz commentedNov 11, 2022
Thanks for this proposal@dbrumann. In theory this addition looks reasonable ... but the Clock component has just been added. When things are so new, I think it's better to let people use it for some time. If something is annoying and requires a DX improvement, many people will complain about it and some will even report it here. |
wouterj commentedNov 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think it makes sense to add this to the |
nicolas-grekas commentedNov 11, 2022
I'm wondering if it wouldn't make more sense to expose a function that modifies the "now" while still using it as a reference. $this->clock->setTo($this->clock->now()->modify('3 weeks')); If the first example is confusion already, maybe the API is not the best :) What about a modify function instead? functionmodify(string$modifier):void{$this->now =$this->now->modifiy($modifier);} |
OskarStark commentedNov 11, 2022
I think both Lets say you want to test, that on Christmas Eve an email is send and not before, its more handy to use the WDYT? |
GromNaN commentedNov 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The var_dump((newDateTimeImmutable())->modify('2022-12-24'));// 2022-12-24 10:49:34.003795 |
OskarStark commentedNov 11, 2022
TIL; thanks |
dbrumann commentedNov 11, 2022
With |
nicolas-grekas commentedNov 11, 2022
True. |
dbrumann commentedNov 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I used |
GromNaN commentedNov 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
To fuel the discussion, I'm using something similar to the new clock component for some time and I never have to alter the current date after initialization.
|
setTo() in MockClocksetTo() in MockClockmodify() in MockClockOskarStark commentedNov 11, 2022
Can you please add a CHANGELOG entry? |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just some minor details and good to me thanks.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedNov 12, 2022
Can you please also update the PR description btw? |
8ebc7f8 toea442bdCompareea442bd to4d3ce8fComparenicolas-grekas commentedNov 14, 2022
Thank you@dbrumann. |
Uh oh!
There was an error while loading.Please reload this page.
This is mostly for DX, especially when handling longer sleep periods and when having to change the time multiple times on a single MockClock-instance, e.g. when using a service from a booted kernel in functional tests.
This PR provides a way to modify the MockClock's current time by specifying either a relative string like "+2 days", which will forward the current clock-time by 2 days, or an absolute datetime like "2112-09-17 23:53:03". It uses
DateTimeImmutable::modify()and supports the same modifiers.Example usage comparison between the already available sleep and modify: