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

review all uses of convertAlldayUtcToLocal #1470

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

Conversation

lmamane
Copy link

@lmamane lmamane commented Oct 6, 2023

this pull request is one commit above #1469; it tries to proactively fix the same underlying issue everywhere convertAlldayUtcToLocal is used in a suspicious way.

Utils.convertAlldayUtcToLocal(
event.endTime, event.endTime.toMillis(), mTimeZone);
final t = new Time();
event.startTime.set(
Copy link
Author

@lmamane lmamane Oct 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one I'm less certain of; I'm not sure what exactly the rest of the code makes with event.startTime and event.endTime; maybe that bit of code should instead be:

long millis = Utils.convertAlldayUtcToLocal(
                                t, event.startTime.toMillis(), mTimeZone);
event.startTime.setTimeZone(mTimeZone);
event.startTime.set(millis);

and the same for event.endTime

@lmamane lmamane force-pushed the cADUTL_robustness_fix branch from 895abe8 to 4169e03 Compare October 6, 2023 15:29
when it looks like the caller does not expect it
@lmamane lmamane force-pushed the cADUTL_robustness_fix branch from 4169e03 to 63a3219 Compare October 24, 2023 02:09
@lmamane
Copy link
Author

lmamane commented Oct 24, 2023

Since #1469 was applied, rebase to that this PR shows as one commit instead of showing commits that are already applied.

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.

1 participant