-
Notifications
You must be signed in to change notification settings - Fork 62
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
Migrate AstRewriterBase
to PIG's VisitorTransform
#356
Conversation
Codecov Report
@@ Coverage Diff @@
## master #356 +/- ##
============================================
- Coverage 82.37% 81.68% -0.69%
+ Complexity 1233 1211 -22
============================================
Files 157 158 +1
Lines 9366 9452 +86
Branches 1526 1537 +11
============================================
+ Hits 7715 7721 +6
- Misses 1192 1264 +72
- Partials 459 467 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Overall LGTM (as it should since each commit was a previously reviewed PR!) but I think we need to add a CompilerPipeline.addPreprocessingStep
overload for PartiqlAst
.
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.
Actually, if you want you can do that addProcessingStep
overload in a PR and merge this one. Approving now so you have that option.
… RewriterTestBase and add depr. Remove depr warning from ExprNodeExtensions
New commit
I felt the I deleted the deprecation warnings in As for the |
Migrates
AstRewriterBase
to PIG'sVisitorTransform
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.