> On Sept. 14, 2013, 7:07 p.m., Swapnil Ghike wrote: > > kafka-patch-review.py, lines 50-51 > > <https://reviews.apache.org/r/14091/diff/11/?file=352200#file352200line50> > > > > Do you intend to save the patch file in current directory? I wonder if > > we should delete it, because it is more of a side effect with this tool.
I don't think we need to cleanup after this tool since patch files are very small in size. Users can easily do that. > On Sept. 14, 2013, 7:07 p.m., Swapnil Ghike wrote: > > kafka-patch-review.py, line 77 > > <https://reviews.apache.org/r/14091/diff/11/?file=352200#file352200line77> > > > > Not sure if there should be a space after -r, or if should be --rb= Every command line option has a short-hand version that can be specified using one "-". This is true of most command line tools. You can read the short and full versions of all options using --help > On Sept. 14, 2013, 7:07 p.m., Swapnil Ghike wrote: > > kafka-patch-review.py, lines 41-43 > > <https://reviews.apache.org/r/14091/diff/11/?file=352200#file352200line41> > > > > This is definitely useful. But I can specify versions out of order and > > confuse the reviewer. Is it possible to read the versions attached on the > > JIRA, and auto-increment the latest version by one? > > > > Wondering if there is a jira.get_attachments() function.. Now that I think more about version, I doubt it is useful for the user to specify it. If it is not completely automated like you said, users could make mistakes and get the versions incorrect. Auto incrementing by reading from the jira is overly complicated. I think a <jira>-<timestamp>.patch is a better approach. That way it is completely automated and easy to reason about > On Sept. 14, 2013, 7:07 p.m., Swapnil Ghike wrote: > > kafka-patch-review.py, line 22 > > <https://reviews.apache.org/r/14091/diff/11/?file=352200#file352200line22> > > > > Should we make it default to the current branch? > > I think with multiple branches in progress, it is better for the branch field to be explicit. That way fewer people will make mistakes while uploading patches using the tool. If it is automated, users might make mistakes and upload incorrect patches to JIRA and reviewboard and not realize it. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14091/#review26115 ----------------------------------------------------------- On Sept. 15, 2013, 4:40 p.m., Neha Narkhede wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14091/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2013, 4:40 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1053 > https://issues.apache.org/jira/browse/KAFKA-1053 > > > Repository: kafka > > > Description > ------- > > Addressed Swapnil's review comments - 1) Closed the file handle for the git > command 2) Removed the --version command line option and automated version to > be the timestamp > > > Included Guozhang's suggestion about making --version mandatory if --rb is > specified > > > Included Guozhang's suggestion about making --version mandatory if --rb is > specified > > > Attaching file using jira-python package > > > Changed the script to use the jira-python package, part 2 > > > Changed the script to use the jira-python package > > > Changed the creating a reviewboard comment as suggested by Tejas > > > Included Tejas's comments > > > Changed the post-review tool to publish the rb automatically. Updated the > tool to add a link to the rb in the JIRA > > > publish automatically > > > Patch review tool > > > Diffs > ----- > > .reviewboardrc PRE-CREATION > kafka-patch-review.py PRE-CREATION > > Diff: https://reviews.apache.org/r/14091/diff/ > > > Testing > ------- > > > Thanks, > > Neha Narkhede > >
