Pulling hotfixes down into develop #1257
Conversation
…n the cache to mitigate memory leak instead. test cleanup.
bugfix:missing required prop for track rendering
Hotfix: client-side memory issues
| test('selects a new time zone', () => { | ||
| renderTimeZoneSelect(); | ||
| // @ TODO: Fix this test | ||
| // test('selects a new time zone', () => { |
There was a problem hiding this comment.
I thought we agreed on not commenting this test, but just removing the failing part of it:
expect(setTimeOfDayTimeZone).toHaveBeenCalledWith('America/Guadeloupe');
The test is still useful to make sure the callback is triggered, we just can't tell exactly which timezone will be.
There was a problem hiding this comment.
Agreed. I'll fix this directly in develop.
| return `${src}:${w}:${h}`; | ||
| }; | ||
|
|
||
| export const imgElFromSrc = (src, baseUnit = null) => { |
There was a problem hiding this comment.
Hmm I still see some of the confusing variable names I suggested to change in the feedback of the hotfix PR: w, h, imgElFromSrc
There was a problem hiding this comment.
Hotfixes are urgent code repairs, and I think it's a reasonable perspective to take on general feedback/code improvements separately from hotfixes -- especially for long-lived existing code like this.
There was a problem hiding this comment.
Totally agreed. I think what I mean is not to improve old code like we do with new PRs (boyscout rule). But I notice new code being added as part of the fix, and I would argue that the new code should come with the high standards we have.
That said, given the urgency and the fact that indeed the code of this file is too old and could take a big refactor, I wouldn't block this.
See title