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

lyaml: fix alias serialization #9736

Closed
wants to merge 1 commit into from

Conversation

nshy
Copy link
Contributor

@nshy nshy commented Feb 27, 2024

The #8350 was introduced by the commit b42302f ("lua-yaml: enable aliasing for objects returned by __serialize") so the patch is effectively reversed.

The idea is to call all object __serialize() methods recursively before finding references. In the process we need to keep an eye on reference structure of course.

Closes #8350
Closes #8321
Closes #8310

@nshy nshy requested review from a team as code owners February 27, 2024 11:55
@coveralls
Copy link

coveralls commented Feb 27, 2024

Coverage Status

coverage: 86.929% (-0.04%) from 86.965%
when pulling b606dac on nshy:fix-serialization-anchors
into 94d2813
on tarantool:master
.

The tarantool#8350 was introduced by the commit b42302f ("lua-yaml: enable
aliasing for objects returned by __serialize") so the patch is
effectively reversed.

The idea is to call all object __serialize methods recursively before
finding references. In the process we need to keep an eye on reference
structure of course.

Closes tarantool#8350
Closes tarantool#8321
Closes tarantool#8310

NO_DOC=bugfix
@@ -0,0 +1,3 @@
## bugfix/lua

* Fixed yaml alias serialization (gh-8350, gh-8321, gh-8310).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Fixed yaml alias serialization (gh-8350, gh-8321, gh-8310).
* Fixed serialization of YAML aliases (gh-8350, gh-8321, gh-8310).

Totktonada
Totktonada previously approved these changes Mar 1, 2024
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

Thank you for the decent work!

I've several topics to discuss together. Please, consider them in the discussion threads below.

third_party/lua-yaml/lyaml.cc Show resolved Hide resolved
third_party/lua-yaml/lyaml.cc Show resolved Hide resolved
third_party/lua-yaml/lyaml.cc Show resolved Hide resolved
@Totktonada Totktonada assigned nshy and unassigned Totktonada Mar 1, 2024
@Totktonada Totktonada dismissed their stale review March 1, 2024 15:14

Sent my mistake.

@Totktonada
Copy link
Member

@nshy asked me to send my proposal from this comment as a pull request. It is done in PR #9777.

@nshy
Copy link
Contributor Author

nshy commented Mar 7, 2024

Closed as superseded by PR #9777

@nshy nshy closed this Mar 7, 2024
@nshy nshy deleted the fix-serialization-anchors branch March 7, 2024 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants