Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21615 )
Change subject: IMPALA-13142: [DOCS] Documentation for Impala StateStore & Catalogd HA ...................................................................... Patch Set 1: (14 comments) Looks good to me, some minor issues. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/impala.ditamap File docs/impala.ditamap: http://gerrit.cloudera.org:8080/#/c/21615/1/docs/impala.ditamap@67 PS1, Line 67: impala_HA better to use lowercase letters. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/impala.ditamap@68 PS1, Line 68: impala_HA_statestore better to use lowercase letters. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/impala.ditamap@69 PS1, Line 69: impala_HA_catalog better to use lowercase letters. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA.xml File docs/topics/impala_HA.xml: http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA.xml@21 PS1, Line 21: impala_HA It seems we prefer to use lowercase letters "impala_ha" for the titles. Maybe good to follow it. Also the file name "impala_HA.xml", maybe good to change to "impala_ha.xml" http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_catalog.xml File docs/topics/impala_HA_catalog.xml: http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_catalog.xml@22 PS1, Line 22: impala_HA_catalog better to use lowercase letters for titles. impala_ha_catalog and impala_ha_catalog.xml http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_catalog.xml@23 PS1, Line 23: nit. unnecessary spaces, same below http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_catalog.xml@49 PS1, Line 49: enabling_catalog_HA better to use lowercase letters for ids. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_catalog.xml@63 PS1, Line 63: disabling_catalog_HA better to use lowercase letters for ids. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_catalog.xml@76 PS1, Line 76: monitoring_status_HA better to use lowercase letters for ids. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_statestore.xml File docs/topics/impala_HA_statestore.xml: http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_statestore.xml@22 PS1, Line 22: impala_HA_statestore better to use lowercase letters for titles. impala_ha_statestore and impala_ha_statestore.xml http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_statestore.xml@23 PS1, Line 23: nit. unnecessary spaces, same below http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_statestore.xml@49 PS1, Line 49: enabling_statestore_HA better to use lowercase letters for ids. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_statestore.xml@63 PS1, Line 63: Existing I don't like using 'Existing' and 'New' flags because I doubt this document will update in the next version to remove those terms. It might be better to simply specify which configurations to set. http://gerrit.cloudera.org:8080/#/c/21615/1/docs/topics/impala_HA_statestore.xml@76 PS1, Line 76: disabling_statestore_HA better to use lowercase letters for ids. -- To view, visit http://gerrit.cloudera.org:8080/21615 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8927c9cd61f0274ad91111d6ac4a079f7a563197 Gerrit-Change-Number: 21615 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Sat, 27 Jul 2024 01:44:06 +0000 Gerrit-HasComments: Yes
