Skip to content

[master < T0802-MG] Make temporal types now functions use the query context timestamp#333

Merged
antonio2368 merged 5 commits intomasterfrom
T0802-MG-make-temporal-types-now-functions-use-the-query-context-timestamp
Feb 4, 2022
Merged

[master < T0802-MG] Make temporal types now functions use the query context timestamp#333
antonio2368 merged 5 commits intomasterfrom
T0802-MG-make-temporal-types-now-functions-use-the-query-context-timestamp

Conversation

@kostasrim
Copy link
Contributor

@kostasrim kostasrim commented Jan 24, 2022

[master < Epic] PR

  • Check, and update documentation if necessary
  • Update changelog
  • Write E2E tests
  • Compare the benchmarking results between the master branch and the Epic branch

[master < Task] PR

  • Check, and update documentation if necessary
  • Update changelog

@kostasrim kostasrim marked this pull request as ready for review February 1, 2022 10:21
@kostasrim kostasrim force-pushed the T0802-MG-make-temporal-types-now-functions-use-the-query-context-timestamp branch from ee712c4 to 086f6aa Compare February 2, 2022 13:13
Comment on lines -109 to +89
const auto utc_today = GetUtcFromSystemClockOrThrow();
return LocalDateTime({TMYearToUtcYear(utc_today.tm_year), TMMonthToUtcMonth(utc_today.tm_mon), utc_today.tm_mday},
{utc_today.tm_hour, utc_today.tm_min, utc_today.tm_sec});
namespace chrono = std::chrono;
auto ts = chrono::time_point_cast<chrono::microseconds>(chrono::system_clock::now());
return LocalDateTime(ts.time_since_epoch().count());
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this return UTC time? I guess no, but you opted from chrono to be consistent with the timestamp thingy. And that is fine, however I think these function should be renamed, because in this case their name is misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right and I will very happily rename it!

@kostasrim kostasrim self-assigned this Feb 2, 2022
@kostasrim kostasrim force-pushed the T0802-MG-make-temporal-types-now-functions-use-the-query-context-timestamp branch from 086f6aa to 15acbb1 Compare February 3, 2022 11:40
@kostasrim
Copy link
Contributor Author

kostasrim commented Feb 3, 2022

Don't merge. Need to update the change log and rebase.

@antonio2368
Copy link
Contributor

Can this be merged now?

@kostasrim
Copy link
Contributor Author

memgraph/docs#299

@antonio2368 antonio2368 merged commit 4fd8bdc into master Feb 4, 2022
@antonio2368 antonio2368 deleted the T0802-MG-make-temporal-types-now-functions-use-the-query-context-timestamp branch February 4, 2022 11:57
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.

4 participants