----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50982/#review145631 -----------------------------------------------------------
Fix it, then Ship it! Overall, the fix looks good. Some comments below. beeline/src/java/org/apache/hive/beeline/TableOutputFormat.java (line 63) <https://reviews.apache.org/r/50982/#comment211946> nit, use the formatter to correct the spaces beeline/src/test/org/apache/hive/beeline/TestTableOutputFormat.java (line 35) <https://reviews.apache.org/r/50982/#comment211945> I don't think the author comment is required. If you use Eclipse, I found importing the coding-style "eclipse-styles.xml" in dev-support directory helpful to format the code using the coding conventions. This will also remove all the trailing spaces (seen in red below) beeline/src/test/org/apache/hive/beeline/TestTableOutputFormat.java (line 100) <https://reviews.apache.org/r/50982/#comment211950> Is it possible to avoid hardcoding this string? If in the future beelineOpts.maxColoumnWidth default values are changed, this test might fail. I think it will be more robust to determine this string programatically using the values in mockResultData and value of maxColumnWidth. - Vihang Karajgaonkar On Aug. 12, 2016, 3:16 p.m., Miklos Csanady wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50982/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2016, 3:16 p.m.) > > > Review request for hive, Peter Vary and Sergio Pena. > > > Bugs: HIVE-14345 > https://issues.apache.org/jira/browse/HIVE-14345 > > > Repository: hive-git > > > Description > ------- > > Fixed output table formatting header and footer lines. > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/TableOutputFormat.java 2753568 > beeline/src/test/org/apache/hive/beeline/TestTableOutputFormat.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/50982/diff/ > > > Testing > ------- > > See attached Unit testClass. > After building with patch, the bug eliminated. > > > File Attachments > ---------------- > > corrected version > > https://reviews.apache.org/media/uploaded/files/2016/08/12/aed736a0-95f1-4f5b-9bda-4db767911065__HIVE-14345.patch > > > Thanks, > > Miklos Csanady > >
