-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enhancement/prune properties root node #4212
Enhancement/prune properties root node #4212
Conversation
…hancement/prune-properties-root-node
Codecov Report
@@ Coverage Diff @@
## master #4212 +/- ##
==========================================
+ Coverage 84.89% 84.93% +0.04%
==========================================
Files 1326 1326
Lines 131683 131745 +62
==========================================
+ Hits 111793 111899 +106
+ Misses 19890 19846 -44
Continue to review full report at Codecov.
|
void PrunePropertiesVisitor::visitCurrent(Project *node) { | ||
// TODO won't use properties of not-root Project | ||
// bool used = used_; | ||
used_ = false; |
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.
When would used_
be true?
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.
In first plan node (root node)
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.
LGTM
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Close #4156
Description:
Previous prune properties require the root node to be
Project
, this PR fix it and let it could be others.How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: