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.

Sign up for GitHub

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

Jump to bottom

Make the Value private #194

Merged
merged 6 commits into from
Jan 7, 2024
Merged

Make the Value private #194

merged 6 commits into from
Jan 7, 2024

Conversation

robUx4
Copy link
Contributor

@robUx4 robUx4 commented Dec 29, 2023 •
edited
Loading

This is how it was before the EbmlElementDefaultSameStorage() split.

Draft on top of #186#200

@robUx4 robUx4 added api-break breaks the API (e.g. programs using it will have to adjust their source code) abi-break breaks the ABI (e.g. programs linked against the library have to be recompiled) labels Dec 29, 2023
@robUx4 robUx4 force-pushed the value_private branch 2 times, most recently from 0a3e6e1 to b217598 Compare January 1, 2024 15:44
mbunkus
mbunkus requested changes Jan 1, 2024
View reviewed changes
Copy link
Contributor

@mbunkus mbunkus left a comment

Choose a reason for hiding this comment

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

Apart from the obvious bug this PR is fine with me. Thanks!

src/EbmlDate.cpp Outdated Show resolved Hide resolved
@robUx4
use SetValue in ReadData()
03d31fd 
So it sets the value and whatever state is updated.
@robUx4
use GetValue in RenderData()
3cd8522
@robUx4 robUx4 marked this pull request as ready for review January 6, 2024 08:06
mbunkus
mbunkus requested changes Jan 6, 2024
View reviewed changes
Copy link
Contributor

@mbunkus mbunkus left a comment

Choose a reason for hiding this comment

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

Only that one cosmetic change I'd like to see. Apart from that it works fine, tests run fine. If all you do is change the function names, then I'll be fine with you merging right away.

ebml/EbmlDate.h Outdated Show resolved Hide resolved
@robUx4
add functions to convert EBML dates to/from UNIX EPOCH
355c34c
@robUx4
use internal converter functions for EBML dates
12f3585
@robUx4
use SetValue/GetValue in EbmlDate
19e7891 
And avoid an infinite loop by calling the proper parent setter/getter.
@robUx4
make the EbmlElementDefaultSameStorage Value private
74f0a06 
This is how it was before the EbmlElementDefaultSameStorage split.
@robUx4 robUx4 merged commit 3a1757e into Matroska-Org:master Jan 7, 2024
15 checks passed
@robUx4 robUx4 deleted the value_private branch January 7, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbunkus mbunkus mbunkus requested changes

Assignees
No one assigned
Labels
abi-break breaks the ABI (e.g. programs linked against the library have to be recompiled) api-break breaks the API (e.g. programs using it will have to adjust their source code)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@robUx4 @mbunkus