[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/984 Discussed in person. Conflicts are resolved. We have a plan for merging with other in-flight PRs. +1 (again) ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user ilooner commented on the issue: https://github.com/apache/drill/pull/984 Resolved conflicts. ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/984 @ilooner as far as know you are doing presentation today connected with this pull request. If overall approach will be accepted, then we'll try to merge this pull request. Please resolve the conflicts as well. ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user ilooner commented on the issue: https://github.com/apache/drill/pull/984 @arina-ielchiieva I can put **DRILL-5783** in a separate PR **DRILL-5841** and **DRILL-5894** are however tightly coupled and must be in the same PR. Unfortunately **DRILL-5841** and **DRILL-5894** touched many files because all the unit tests were updated so the PR will still remain large. ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/984 @ilooner changes in 361 files seems to be too much :) I suggest you split this PR at least at three according to the addressed issues. So we try to merge smaller chunks. ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user ilooner commented on the issue: https://github.com/apache/drill/pull/984 After squashing the commits and rebasing I noticed the windows functional tests were failing. The issue was caused by replacing the '/' constant in FileUtils (now renamed to DrillFileUtils) in **ClassTransformer** and **FunctionInitializer** with File.separator. This broke the build because File.separator is '\' on windows and in the context of **ClassTransformer** and **FunctionInitializer** the separator is used to look things up in the classpath. Looking things up in the classpath requires '/' on both windows and linux, hence I added back the '/' constant to DrillFileUtils along with a comment explaining its purpose and used it in **ClassTransformer** and **FunctionInitializer**. *Note:* this was a minor fix that impacted only a few lines. ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user ilooner commented on the issue: https://github.com/apache/drill/pull/984 @paul-rogers This is ready for another round of review. I have also responded to / addressed your comments. ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user ilooner commented on the issue: https://github.com/apache/drill/pull/984 @paul-rogers Applied / responding to comments. Also removed RecordBatchBuilder and used RowSetBuilder. ---
[GitHub] drill issue #984: DRILL-5783 Made a unit test for generated Priority Queue. ...
Github user ilooner commented on the issue: https://github.com/apache/drill/pull/984 @paul-rogers ---
