[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user vrozov commented on the issue: https://github.com/apache/drill/pull/1099 @reudismam Travis fails in other PRs as well. See #1105. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 Only pass Travis CI by removing the edits to SSLConfigClient.java ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 I have squashed the commits, but Iâm getting an error in Travis CI similar to the previous one when I reverted some changes. Column a-offsets of type UInt4Vector: Offset (0) must be 0 but was 1 ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1099 Well, you can always use force push to override your previous changes or even replace your remote branch with new local. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 Maybe it has not worked as expected. It squashed the commit (first commit), but as the commit mix commits from other persons, they come together. Maybe it will be the case of creating a patch file for the desired commit and apply this patch to a new pull request. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user vrozov commented on the issue: https://github.com/apache/drill/pull/1099 @reudismam For drill, merging/squashing on github is not an option (due to the way how Apache Drill git is setup). Please rebase and squash. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 Since I have to resolve some conflicts after applying the edit, squashing could be tricky because the commits are non-consecutive. I allowed the merging using the squashing option in the forked project setting. Thus, when someone merges the pull request, he or she can decide to squash the commits.  ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user vrozov commented on the issue: https://github.com/apache/drill/pull/1099 LGTM, please squash. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1099 LGTM, @vrozov? ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1099 @reudismam I guess you can apply back your change for `SSLConfigClient.java` class. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user vrozov commented on the issue: https://github.com/apache/drill/pull/1099 Travis CI failed due to a known issue `TestSortImpl.testLargeBatch:502->runJumboBatchTest:475->runLargeSortTest:454 Value of 1:0 expected:<0> but was:<1>` unrelated to the SSLConfigClient.java changes. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 I reverted the edit because the code does not pass in Travis CI. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user vrozov commented on the issue: https://github.com/apache/drill/pull/1099 What was the reason to revert the change in SSLConfigClient.java? ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1099 @reudismam you can leave `valueOf` for Double and Float for consistency. There are two other comments @vrozov has left. Could you please address hem as well? They are not directly related to your changes but will make Drill code better. Also please squash commits into one. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 Conflicts resolved. Do you want to revert the edits to Double and Float? I think it is ok for consistency. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1099 @reudismam could you please address code review comments so changes can be pushed into master? Also please resolve merge conflict. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 Reverted changes to classes in the protocol package. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1099 @reudismam classes in protocol package are generated using .proto files, you can not change them. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1099 @reudismam the idea of the fix seems reasonable but there are many other places in code where constructor is used instead of `valueOf`. I suggest you revisit all classes rather then doing partial fix. ---
[GitHub] drill issue #1099: DRILL-6106: Use valueOf method instead of constructor sin...
Github user reudismam commented on the issue: https://github.com/apache/drill/pull/1099 I have edited the PR to include the Jira prefix. ---
