impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5110: Add deb support to dump breakpad
Date Fri, 24 Mar 2017 12:42:42 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-5110: Add deb support to

Patch Set 2:


Thank you for the review. Please see my comments and PS2.
File bin/

Line 115:   parser.add_argument('-r', '--pkg', '--rpm', help="""RPM/DEB file containing the
> Instead of adding a flag for every package type, should we just add a gener
Done. I changed the flag name so we don't have to worry about compatibility in the future.
However I kept the help string as I have only seen RPM and DEB being used to package Impala
and the code below won't work with anything else.

Line 117:   parser.add_argument('-s', '--symbol_pkg', '--debuginfo_rpm', help="""RPM/DEB file
> Same here - --symbol_pkg or similar?

Line 208:'Extracting to %s: %s' % (tmp_dir, pkg))
> Mention the output directory? Might be helpful for debugging if it fails.
Done. The output directory will be removed in case of failure though.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I524d2fe4660551c2fe4ff190b7e5bbb33d986b10
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message