> On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > common/src/main/java/org/apache/drill/common/config/package-info.java, line > > 21 > > <https://reviews.apache.org/r/29278/diff/3/?file=823063#file823063line21> > > > > Is nesting the salient character for this package? Is there anything > > else that would be surprising?
Added some more information about how config files are persisted and a referral to the class responsible for finding the files on the classpath/resolving configuration precedence. > On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > common/src/main/java/org/apache/drill/common/exceptions/package-info.java, > > line 19 > > <https://reviews.apache.org/r/29278/diff/3/?file=823064#file823064line19> > > > > Probably not even needed. The name and the content speak for > > themselves. > > > > They may be something that is not said by these contents, but the mere > > fact that these are exceptions is, so to say, unexceptional. Fair enough, for consistency I just documented it. > On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > common/src/main/java/org/apache/drill/common/expression/package-info.java, > > line 19 > > <https://reviews.apache.org/r/29278/diff/3/?file=823067#file823067line19> > > > > Too thin to say anything here. What is a logical expression? An > > expression with nothing but booleans? Or an expression in the logical plan. > > > > Perhaps clarification by amplification is in order. added additional clarification > On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > common/src/main/java/org/apache/drill/common/expression/visitors/package-info.java, > > line 19 > > <https://reviews.apache.org/r/29278/diff/3/?file=823069#file823069line19> > > > > Used in what context? Who calls these? When are the inserted? Added some information about where the visitors are used. > On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > common/src/main/java/org/apache/drill/common/logical/FormatPluginConfig.java, > > line 27 > > <https://reviews.apache.org/r/29278/diff/3/?file=823071#file823071line27> > > > > Suggest "Formats are file formats that have nothing to do with how the > > file is stored, such as CSV or protobuf. Updated comments to clarify the relationship between format plugins and storage plugins. > On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > common/src/main/java/org/apache/drill/common/logical/FormatPluginConfigBase.java, > > line 27 > > <https://reviews.apache.org/r/29278/diff/3/?file=823072#file823072line27> > > > > A tiny bit more would be nice. "To use this base class, sub-class it > > and define .... Watch out fo r ..." This actually isn't being used as the base class for anything. Opened a JIRA to review the current structure and removed the docs for now. https://issues.apache.org/jira/browse/DRILL-2076 > On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/coord/package-info.java, > > line 19 > > <https://reviews.apache.org/r/29278/diff/3/?file=823086#file823086line19> > > > > Need a bit more here. Could have a pointer to what sort of > > coordination is done and a description of why. > > > > Is Curator used? Added curator to the comment. I don't really think we maintain much beside cluster membership using zookeeper. I think this is a reasonable descritpion for now. > On Jan. 16, 2015, 11:23 p.m., Ted Dunning wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/package-info.java, line > > 22 > > <https://reviews.apache.org/r/29278/diff/3/?file=823091#file823091line22> > > > > This is a top-level description that should not quote so much as > > redirect the reader. A link to the architecture home page is sufficient > > for that. Links to the key pieces of code to read next would be very > > helpful as well. I have fixed the comment to include a link. I started writing a more complete intro for new developers, but it was getting a bit long without covering the breadth I wanted. I have opened a JIRA for better starting documentation for new developers here as well as on the Wiki. I would like to get this patch in sooner to encourge others to write docs with a consistent style and format, but I will be trying to fit in an improvement to this documentation soon. https://issues.apache.org/jira/browse/DRILL-2077 - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29278/#review68501 ----------------------------------------------------------- On Jan. 27, 2015, 1:36 a.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29278/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2015, 1:36 a.m.) > > > Review request for drill, Daniel Barclay and Jacques Nadeau. > > > Bugs: Drill-1904 > https://issues.apache.org/jira/browse/Drill-1904 > > > Repository: drill-git > > > Description > ------- > > Add package level documentation to the sources of Drill > > > Diffs > ----- > > common/src/main/java/org/apache/drill/common/config/package-info.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/exceptions/package-info.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/expression/aggregate/BasicAggregates.java > 5f18aaf > > common/src/main/java/org/apache/drill/common/expression/fn/package-info.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/expression/package-info.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/expression/types/package-info.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/expression/visitors/package-info.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/graph/package-info.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/logical/FormatPluginConfig.java > 936e4f2 > > common/src/main/java/org/apache/drill/common/logical/FormatPluginConfigBase.java > 0704085 > common/src/main/java/org/apache/drill/common/logical/data/package-info.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/logical/data/visitors/package-info.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/logical/package-info.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/package-info.java PRE-CREATION > common/src/main/java/org/apache/drill/common/types/package-info.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/util/package-info.java > PRE-CREATION > > contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/package-info.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/cache/package-info.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/client/package-info.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/package-info.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/compile/package-info.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/package-info.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/coord/local/package-info.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/coord/package-info.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/package-info.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/disk/package-info.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/package-info.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/exception/package-info.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/package-info.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29278/diff/ > > > Testing > ------- > > Just comments, no code changes > > > Thanks, > > Jason Altekruse > >
