----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/400/#review189 -----------------------------------------------------------
http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java <https://reviews.apache.org/r/400/#comment354> expandMacros should probably return a Reader. Or this replacement code should be put in a local method to reduce duplication. http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g <https://reviews.apache.org/r/400/#comment355> Is there a way to avoid duplication here ? http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java <https://reviews.apache.org/r/400/#comment356> What does this do ? http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java <https://reviews.apache.org/r/400/#comment357> What about ".foo/..." ? What about "~" ? http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java <https://reviews.apache.org/r/400/#comment359> the error message should contain the actual mismatching values to help with debugging http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java <https://reviews.apache.org/r/400/#comment360> Catching individual exception may help with better error messages http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java <https://reviews.apache.org/r/400/#comment358> do we need specific exception types here? http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java <https://reviews.apache.org/r/400/#comment363> Can a Macro call another Macro ? This would be useful. http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java <https://reviews.apache.org/r/400/#comment361> I like the syntax :) http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java <https://reviews.apache.org/r/400/#comment362> what if I call gamma macro_group_and_count_D_0 instead ? Would it fail? http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java <https://reviews.apache.org/r/400/#comment365> pretty cool Does the following work: - Can a Macro call another Macro ? - Can a Macro take a Macro in parameter? - Julien On 2011-02-07 10:13:39, Richard Ding wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/400/ > ----------------------------------------------------------- > > (Updated 2011-02-07 10:13:39) > > > Review request for pig and Julien Le Dem. > > > Summary > ------- > > As production Pig scripts grow longer and longer, Pig Latin has a need to > integrate standard programming techniques of separation and code sharing > offered by functions and modules. A detailed proposal of adding macro > expansion to Pig Latin is posted here: > http://wiki.apache.org/pig/TuringCompletePig > > > This addresses bug PIG-1793. > https://issues.apache.org/jira/browse/PIG-1793 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/pig/trunk/build.xml 1067233 > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/Main.java > 1067233 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroExpansion.g > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/MacroImport.g > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/ParserUtil.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/PigMacro.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g > 1067233 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/scripting/Pig.java > 1067233 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/400/diff > > > Testing > ------- > > > Thanks, > > Richard > >
