Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.

Conversation

@sezruby
Copy link
Collaborator

@sezruby sezruby commented Jan 27, 2021

What is the context for this pull request?

  • Tracking Issue: n/a
  • Parent Issue: n/a
  • Dependencies: n/a

What changes were proposed in this pull request?

Fix a bug in IndexLogEntry.sourceFilesSzieInBytes.
Set.map(_.size).sum returns incorrect results.

Does this PR introduce any user-facing change?

Yes, fixes the bug.

How was this patch tested?

tested by unit tests in #272

@sezruby sezruby self-assigned this Jan 27, 2021
@sezruby sezruby added the bug Something isn't working label Jan 27, 2021
@imback82
Copy link
Contributor

tested by unit tests in #272

Can we add simple tests in this PR? It's weird that tests are done by a different PR.

@imback82
Copy link
Contributor

Set.map(_.size).sum returns incorrect results.

I wasn't aware of this. Could you explain a bit more? Set(1, 2, 3).map(a => a).sum seems fine?

@sezruby
Copy link
Collaborator Author

sezruby commented Jan 27, 2021

Set(((a,1), (b,1)).map(_._2).sum returns 1 it's because map result became Set, not Seq..

@sezruby
Copy link
Collaborator Author

sezruby commented Jan 27, 2021

Can we add simple tests in this PR? It's weird that tests are done by a different PR.

Not sure this PR needs a separate test..

@imback82
Copy link
Contributor

Not sure this PR needs a separate test..

How did you verify the fix then?

@sezruby
Copy link
Collaborator Author

sezruby commented Jan 27, 2021

@imback82 Updated a simple test. Thanks!

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @sezruby!

@imback82
Copy link
Contributor

Set(((a,1), (b,1)).map(_._2).sum returns 1 it's because map result became Set, not Seq..

👍🏼

@imback82 imback82 added this to the January 2021 milestone Jan 27, 2021
@imback82 imback82 merged commit 6a3bf0b into microsoft:master Jan 27, 2021
@sezruby sezruby deleted the sizequickfix branch April 30, 2021 03:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants