> On March 1, 2013, 11:13 p.m., Cheolsoo Park wrote: > > Hi Jonathan, > > > > Overall looks good. My only concern is lack of unit tests. Indeed, you > > converted some test cases in TestBuiltin, but that doesn't seem to cover > > dump, describe, explain, illustrate. I am wondering whether you can add > > basic test cases somewhere. PIG-2994 added TestShortcuts, and that might be > > a good place to add new test cases? > > > > Let me know what you think.
Totally agree. Will do. > On March 1, 2013, 11:13 p.m., Cheolsoo Park wrote: > > src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj, line 250 > > <https://reviews.apache.org/r/9496/diff/2/?file=263924#file263924line250> > > > > Please update (" ")* with (" " | "\t")*. This is fine in interactive > > mode since Grunt shell doesn't allow tab chars. However, it will throw an > > error when executing a Pig script that contains tab chars in batch mode. > > For example, if I have a Pig script called test.pig: > > > > -- tab after fat arrow > > => load '1.txt'; > > dump @; > > > > ./bin/pig -x local test.pig > > > > This fails because of the tab char is not recognized. > > > > But the following script works: > > > > -- tab after equal > > a = load '1.txt'; > > dump a; Good point, will do. I'm much more cargo culting in javacc than antlr :) - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9496/#review17244 ----------------------------------------------------------- On March 1, 2013, 10:32 a.m., Jonathan Coveney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9496/ > ----------------------------------------------------------- > > (Updated March 1, 2013, 10:32 a.m.) > > > Review request for pig, Daniel Dai and Alan Gates. > > > Description > ------- > > https://issues.apache.org/jira/browse/PIG-3136 > > > This addresses bug PIG-3136. > https://issues.apache.org/jira/browse/PIG-3136 > > > Diffs > ----- > > src/org/apache/pig/PigServer.java a46cc2a > src/org/apache/pig/parser/LogicalPlanBuilder.java b705686 > src/org/apache/pig/parser/LogicalPlanGenerator.g 01dc570 > src/org/apache/pig/parser/QueryLexer.g bdd0836 > src/org/apache/pig/parser/QueryParser.g ba2fec9 > src/org/apache/pig/parser/QueryParserDriver.java a250b73 > src/org/apache/pig/tools/grunt/GruntParser.java 2658466 > src/org/apache/pig/tools/pigscript/parser/PigScriptParser.jj 109a3b2 > test/org/apache/pig/test/TestBuiltin.java 9c5d408 > > Diff: https://reviews.apache.org/r/9496/diff/ > > > Testing > ------- > > ant -Dtestcase=TestBuiltin test, and ant test-commit > > I made them TestBuiltin use it > > > Thanks, > > Jonathan Coveney > >
