Skip to content

Conversation

patiencedaur
Copy link
Contributor

Resolves #2509

@github-actions github-actions bot temporarily deployed to branch-gh-2509-datetime-storage July 15, 2022 11:26 Inactive
@github-actions github-actions bot temporarily deployed to branch-gh-2509-datetime-storage July 16, 2022 05:23 Inactive
@github-actions github-actions bot temporarily deployed to branch-gh-2509-datetime-storage July 16, 2022 05:24 Inactive
@patiencedaur patiencedaur marked this pull request as ready for review July 16, 2022 07:52
@patiencedaur patiencedaur requested a review from tsafin July 16, 2022 07:53
@github-actions github-actions bot temporarily deployed to branch-gh-2509-datetime-storage July 16, 2022 07:53 Inactive
Copy link
Contributor

@tsafin tsafin left a comment

Choose a reason for hiding this comment

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

I have no objections about markup refactorings here and there, but datetime doc in the messagepack datamodel descriptions seems like is mismatching their current neighbourhood. And should be moved elsewhere.

+-------------------+-------------------------+--------------------------------+------------------------------+
| scalar | integer | "`number`_" | ``12345`` |
+-------------------+-------------------------+--------------------------------+------------------------------+
| scalar | float 64 (double) | "`number`_" | ``1.2345`` |
Copy link
Contributor

Choose a reason for hiding this comment

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

I do see it's kind of offtopic for the datetime discussion, but still is bothering me: why there are 2 rows for double literals? (LuaJIT ffi cdata and standard Lua number). What we wanted to highlight here? (That we could, depending on value, may end up using either number or extended LuaJIT type? But similarly we would have for integer values, but there are no duplication.)

I'me very much confused here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for noticing that! Tracked in #3040.

* Remove API usage example
* Clarify the status of c-dt library
@github-actions github-actions bot temporarily deployed to branch-gh-2509-datetime-storage July 19, 2022 13:56 Inactive
@github-actions github-actions bot temporarily deployed to branch-gh-2509-datetime-storage July 19, 2022 14:42 Inactive
@patiencedaur patiencedaur force-pushed the gh-2509-datetime-storage branch from be6a300 to 91fc9c5 Compare July 20, 2022 06:23
@github-actions github-actions bot temporarily deployed to branch-gh-2509-datetime-storage July 20, 2022 06:25 Inactive
@patiencedaur patiencedaur merged commit b03db3f into latest Jul 21, 2022
@patiencedaur patiencedaur deleted the gh-2509-datetime-storage branch July 21, 2022 06:09
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.

[3pt] Storage support for datetime values
2 participants