-
Notifications
You must be signed in to change notification settings - Fork 181
Don't report stale outputs if there is newer generation in new_outputs #879
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
Conversation
✅ Deploy Preview for salsa-rs canceled.
|
CodSpeed Performance ReportMerging #879 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me. @ibraheemdev would you mind taking a look
@@ -25,7 +25,12 @@ where | |||
revisions: &mut QueryRevisions, | |||
) { | |||
// Iterate over the outputs of the `old_memo` and put them into a hashset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would find it useful if we could extend the comment so that it explains why we ignore the generation part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Values that are reused with a new generation already take care of cleaning up the previous generation's value, so we were effectively calling remove_stale_output
twice.
Fixes #878