-
Notifications
You must be signed in to change notification settings - Fork 201
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
Add a new edge kind - ParentIterative
#617
Conversation
ParentIterative
e951072
to
08da159
Compare
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.
Minor nits below, plus the offline request to add the negative test case, but otherwise this LGTM!
|
||
|
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.
Are this extra empty lines intentional?
replace_node = "*" | ||
replace = 'new SparkSession.builder()' | ||
|
||
|
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.
Minor nit: but let's make the spacing between rules consistently, either one or two empty lines, but not a mix of the two unless it implies a particular grouping.
src/models/source_code_unit.rs
Outdated
@@ -280,8 +267,8 @@ impl SourceCodeUnit { | |||
stack: &mut VecDeque<(CGPattern, InstantiatedRule)>, | |||
) { | |||
for (scope_level, rules) in next_rules_by_scope { | |||
// Scope level is not "PArent" or "Global" | |||
if ![PARENT, GLOBAL].contains(&scope_level.as_str()) { | |||
// Scope level is not "Parent" or "Global" |
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.
// Scope level is not "Parent" or "Global" | |
// Scope level is not "Parent", "Parent Iterative", or "Global" |
self.rewrites_mut().push(edit.clone()); | ||
debug!( |
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.
Just to confirm, we are intentionally getting rid of this debug log line entirely, right?
Earlier we had
Parent
and this is how it was processed:For
ParentIterative
we do something subtly different. We loop over the ancestors in the outer most loop:And its processed something like :
Why suffix iterative? I thought its keeping to iterative-deepening idea from DFS.
I am open to other names.
But this pattern makes it easy to transform builder patterns (As required by Scala upgrade folks)