Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21646 )
Change subject: IMPALA-13240: Add gerrit comments for Thrift/FlatBuffers changes ...................................................................... IMPALA-13240: Add gerrit comments for Thrift/FlatBuffers changes Adds gerrit comments for changes in Thrift/FlatBuffers files that could break the communication between impalad and catalogd/statestore during upgrade. Basically, only new optional fields can be added in Thrift protocol. For Flatbuffers schemas, we should only add new fields at the end of a table definition. Adds a new option (--revision) for critique-gerrit-review.py to specify the revision (HEAD or a commit, branch, etc). Also adds an option (--base-revision) to specify the base revision for comparison. To test the script locally, prepare a virtual env with the virtualenv package installed: virtualenv venv source venv/bin/activate pip install virtualenv Then run the script with --dryrun: python bin/jenkins/critique-gerrit-review.py --dryrun --revision effc9df93 Limitations - False positive in cases that add new Thrift structs with required fields and only use those new structs in new optional fields, e.g. effc9df93 and 72732da9d. - Might have false positive results on reformat changes due to simple string checks, e.g. 91d8a8f62. - Can't check incompatible changes in FlatBuffers files. Just add general file level comments. We can integrate DUPCheck in the future to parse the Thrift/FlatBuffers files to AST and compare the AST instead. https://github.com/jwjwyoung/DUPChecker Tests: - Verified incompatible commits like 012996a06 and 65094a74f. - Verified posting Gerrit comments from local env using my username. Change-Id: Ib35fafa50bfd38631312d22464df14d426f55346 Reviewed-on: http://gerrit.cloudera.org:8080/21646 Reviewed-by: Riza Suminto <[email protected]> Tested-by: Quanlong Huang <[email protected]> --- M bin/jenkins/critique-gerrit-review.py M common/thrift/Data.thrift 2 files changed, 211 insertions(+), 26 deletions(-) Approvals: Riza Suminto: Looks good to me, approved Quanlong Huang: Verified -- To view, visit http://gerrit.cloudera.org:8080/21646 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib35fafa50bfd38631312d22464df14d426f55346 Gerrit-Change-Number: 21646 Gerrit-PatchSet: 9 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]>
