----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52570/#review163177 -----------------------------------------------------------
docs/learn/documentation/versioned/hdfs/consumer.md (line 22) <https://reviews.apache.org/r/52570/#comment234613> Re-word to be concise. Avro encoded records are supported out of the box and it is easy to extend to support other formats (plain text, csv, json etc). `in the future` is probably not needed. docs/learn/documentation/versioned/hdfs/consumer.md (line 30) <https://reviews.apache.org/r/52570/#comment234621> nit: How do new lines look when rendered in the web-page? Do they appear like paragraphs? If so, do you think this can be made a single paragraph? (instead of 3). docs/learn/documentation/versioned/hdfs/consumer.md (line 60) <https://reviews.apache.org/r/52570/#comment234634> typo here task.inputs docs/learn/documentation/versioned/hdfs/consumer.md (line 68) <https://reviews.apache.org/r/52570/#comment234633> Would be good to provide dummy values? For instance, the above property (`whitelist`) uses it consistently. docs/learn/documentation/versioned/hdfs/consumer.md (line 76) <https://reviews.apache.org/r/52570/#comment234632> Redundant with previous line. Can potentially abridge? docs/learn/documentation/versioned/hdfs/consumer.md (line 77) <https://reviews.apache.org/r/52570/#comment234635> contains both a '.' and a ':'. Not sure if both are required. docs/learn/documentation/versioned/hdfs/consumer.md (line 81) <https://reviews.apache.org/r/52570/#comment234640> Use newlines after defining properties consistently. docs/learn/documentation/versioned/hdfs/consumer.md (line 83) <https://reviews.apache.org/r/52570/#comment234636> Can provide a dummy value here consistently? docs/learn/documentation/versioned/hdfs/consumer.md (line 89) <https://reviews.apache.org/r/52570/#comment234637> Use periods consistently. For example, the above section ended with a period. docs/learn/documentation/versioned/hdfs/consumer.md (line 92) <https://reviews.apache.org/r/52570/#comment234639> nit: Use capitalizations consistently 1. Is `id` of any significance here? docs/learn/documentation/versioned/hdfs/consumer.md (line 93) <https://reviews.apache.org/r/52570/#comment234654> Please provide a value. docs/learn/documentation/versioned/jobs/configuration-table.html (line 1819) <https://reviews.apache.org/r/52570/#comment234645> Thanks for calling out the trade-offs in the docs. docs/learn/documentation/versioned/jobs/configuration-table.html (line 1832) <https://reviews.apache.org/r/52570/#comment234647> Do we allow any other `partitioner` other than the defaultPartitioner? If not, it feels slightly unconventional to have `default` in the property name? docs/learn/documentation/versioned/jobs/configuration-table.html (line 1839) <https://reviews.apache.org/r/52570/#comment234641> Is the group identifier the partition name? Is `id` a reserved term? Do we always expect `[id]`? docs/learn/documentation/versioned/jobs/configuration-table.html (line 1840) <https://reviews.apache.org/r/52570/#comment234649> Thanks for adding the example. Re-word docs. That you want to organize into `three` partitions as ... docs/learn/documentation/versioned/jobs/configuration-table.html (line 1842) <https://reviews.apache.org/r/52570/#comment234650> Is this the name of the class? I'm still not clear how this is used? docs/learn/documentation/versioned/jobs/configuration-table.html (line 1847) <https://reviews.apache.org/r/52570/#comment234642> The other minor observation is that the camel case naming scheme is slightly inconsistent with what we have for our public configs. For instance, job.coordinator. monitor-partition-change follows a different convention. I'd really prefer consistency in our config scheme. Navina or Prateek thoughts? docs/learn/documentation/versioned/jobs/configuration-table.html (line 1849) <https://reviews.apache.org/r/52570/#comment234643> typo: be default ? Did you mean By default? docs/learn/documentation/versioned/jobs/configuration-table.html (line 1850) <https://reviews.apache.org/r/52570/#comment234644> Typo: Did you want to use two periods `..`? - Jagadish Venkatraman On Jan. 26, 2017, 6:47 p.m., Hai Lu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52570/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2017, 6:47 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-1025 > https://issues.apache.org/jira/browse/SAMZA-1025 > > > Repository: samza > > > Description > ------- > > documentation for hdfs system consumer > > > Diffs > ----- > > docs/learn/documentation/versioned/hdfs/consumer.md PRE-CREATION > docs/learn/documentation/versioned/hdfs/producer.md > b0e936f5b0a9c945ea7f02bfc2536ef50f017bf6 > docs/learn/documentation/versioned/index.html > d0b14ece94341e2cb937cf32db480e69f93303c2 > docs/learn/documentation/versioned/jobs/configuration-table.html > ba5ebbc54b5c64f82f35ed781dad7023a8f920e1 > > Diff: https://reviews.apache.org/r/52570/diff/ > > > Testing > ------- > > N/A > > > Thanks, > > Hai Lu > >
