Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Bugs in array trimming, more code comments #172

Merged
merged 4 commits into from
Jan 30, 2019
Merged

Conversation

untitaker
Copy link
Member

No description provided.

Copy link
Member

@mitsuhiko mitsuhiko left a comment

Choose a reason for hiding this comment

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

lgtm. I want to double check though if the stack is managed correctly in all cases. (newtypes stuff).

@@ -432,6 +432,7 @@ pub struct MetaInner {

impl MetaInner {
pub fn is_empty(&self) -> bool {
// TODO(jan): Evaluate which meta info is relevant enough to keep if the value is gone.
Copy link
Member

Choose a reason for hiding this comment

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

@mitsuhiko @untitaker This is for a follow-up PR. I would argue that we should always be able to add original_length without worrying about serializing a Meta node. If only original_length is set but no errors or remarks, then no meta tree should be serialized for this path. This would slim down meta generated for trimmed structures.

@codecov-io

This comment has been minimized.

Copy link
Member

@mitsuhiko mitsuhiko left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@untitaker untitaker merged commit b0a0a9c into master Jan 30, 2019
@untitaker untitaker deleted the fix/trimming-bugs branch January 30, 2019 08:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants