> On April 28, 2012, 12:32 a.m., Alan Gates wrote: > > There are quite a few spacing issues, like maybe you're mixing tabs and > > spaces or something. The policy in HCat code is to use only spaces.
In HCATALOG-424.patch, Sushanth replaced all the tabs and standardized the spacing and line breaks. > On April 28, 2012, 12:32 a.m., Alan Gates wrote: > > trunk/src/java/org/apache/hcatalog/data/HCatRecord.java, line 55 > > <https://reviews.apache.org/r/4921/diff/1/?file=105159#file105159line55> > > > > "at the indicated position in the record" would be better than "at a > > specified index" Done. > On April 28, 2012, 12:32 a.m., Alan Gates wrote: > > trunk/src/java/org/apache/hcatalog/data/schema/HCatFieldSchema.java, line 32 > > <https://reviews.apache.org/r/4921/diff/1/?file=105161#file105161line32> > > > > What do the default values mean in this enum list? These look like the > > defaults from Java. But HCat/Hive doesn't have a notion of default values. > > The ranges are nice, we should remove the default values. They were defaults from Java, indeed. The replacement patch gets rid of them. > On April 28, 2012, 12:32 a.m., Alan Gates wrote: > > trunk/src/java/org/apache/hcatalog/data/transfer/ReaderContext.java, line 82 > > <https://reviews.apache.org/r/4921/diff/1/?file=105166#file105166line82> > > > > These comments don't look like they match with the function. Changed the description of writeExternal(ObjectOutput out) from "Save the ReaderContext contents" to "Write the input splits used by this reader to an external location." > On April 28, 2012, 12:32 a.m., Alan Gates wrote: > > trunk/src/java/org/apache/hcatalog/data/transfer/ReaderContext.java, line 96 > > <https://reviews.apache.org/r/4921/diff/1/?file=105166#file105166line96> > > > > These comments don't look like they match with the function. Changed the description of readExternal(ObjectInput in) from "Restore the ReaderContext contents" to "Read an external list of input splits to restore the contents of this ReaderContext." > On April 28, 2012, 12:32 a.m., Alan Gates wrote: > > trunk/src/java/org/apache/hcatalog/data/HCatRecord.java, line 62 > > <https://reviews.apache.org/r/4921/diff/1/?file=105159#file105159line62> > > > > Should note that after this call will copy the pointers to the data, > > not the data itself. Should also note that this call will obliterate > > anything currently in this record. > > Clarification, please. What does "after this" mean, or is "after" a typo? Does this wording work? -- "Abstract method to copy an HCatRecord to the current HCatRecord. This call copies the pointers to the data, not the data itself. Note that this call will obliterate anything currently in this record." > On April 28, 2012, 12:32 a.m., Alan Gates wrote: > > trunk/src/java/org/apache/hcatalog/data/transfer/WriterContext.java, line 30 > > <https://reviews.apache.org/r/4921/diff/1/?file=105168#file105168line30> > > > > Should read "slave nodes for writing" rather than "the writer" Done. - Lefty ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4921/#review7339 ----------------------------------------------------------- On April 28, 2012, 12:13 a.m., Alan Gates wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4921/ > ----------------------------------------------------------- > > (Updated April 28, 2012, 12:13 a.m.) > > > Review request for hcatalog. > > > Description > ------- > > Javadoc improvements > > > This addresses bug HCATALOG-381. > https://issues.apache.org/jira/browse/HCATALOG-381 > > > Diffs > ----- > > trunk/src/java/org/apache/hcatalog/data/HCatRecord.java 1311550 > trunk/src/java/org/apache/hcatalog/data/HCatRecordable.java 1311550 > trunk/src/java/org/apache/hcatalog/data/schema/HCatFieldSchema.java 1311550 > trunk/src/java/org/apache/hcatalog/data/schema/HCatSchema.java 1311550 > trunk/src/java/org/apache/hcatalog/data/schema/HCatSchemaUtils.java 1311550 > trunk/src/java/org/apache/hcatalog/data/transfer/EntityBase.java 1311550 > trunk/src/java/org/apache/hcatalog/data/transfer/ReadEntity.java 1311550 > trunk/src/java/org/apache/hcatalog/data/transfer/ReaderContext.java 1311550 > trunk/src/java/org/apache/hcatalog/data/transfer/WriteEntity.java 1311550 > trunk/src/java/org/apache/hcatalog/data/transfer/WriterContext.java 1311550 > trunk/src/java/org/apache/hcatalog/data/transfer/state/StateProvider.java > 1311550 > > Diff: https://reviews.apache.org/r/4921/diff/ > > > Testing > ------- > > > Thanks, > > Alan Gates > >
