[PATCH] D30905: stable-merge-request.sh: Add a script for submitting merge requests via bugzilla

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

One thing I just realized: the bugzilla-cli package, at least for Ubuntu, seems only available in the most recent versions: http://packages.ubuntu.com/search?keywords=bugzilla-cli (similar for Debian: https://packages.debian.org/search?keywords=bugzilla-cli)

At least for my work and home machines, that makes it more impractical to use, and I'm guessing that might apply to many other developers.

There seems to be a tool called "bugz" which is more widely available: http://packages.ubuntu.com/search?keywords=bugz
Maybe the script should use that instead, or both?



================
Comment at: utils/release/merge-request.sh:2
+#!/bin/bash
+#===-- stable-merge-request.sh - Test the LLVM release candidates ----------===#
+#
----------------
nit: drop/change "Test the LLVM release candidates"


================
Comment at: utils/release/merge-request.sh:35
+  echo " -r NUM                  Revision number to merge (e.g. 1234567)."
+  echo " -project PROJECT        Revision project (e.g. clang, llvm, etc.)"
+  echo "                         use -project help for a full list of "
----------------
What does "Revision project" mean?

This seems to refer to the bugzilla Product field. Do we really care about that? Maybe to simplify this, both for the script and for the user, we could just file these bugs against the "new-bugs" product.


================
Comment at: utils/release/merge-request.sh:44
+  echo " -bugzilla-bin PATH      Path to bugzilla binary (optional)."
+  echo " -assign-to-email EMAIL  Assign bug to user with EMAIL (optional)."
+  echo " -disable-verify         Don't verify project/component (optional)."
----------------
Since "-user" is not spelled "-user-email", I think this could be spelled simply "-assign-to".


================
Comment at: utils/release/merge-request.sh:224
+# Check for duplicate:
+check_duplicates=`$BUGZILLA_CMD query --url $revision`
+
----------------
This could match other bugs too.. maybe the query should be narrower?


================
Comment at: utils/release/merge-request.sh:234
+commit_summary=''
+commit_msg=`svn log -r $revision`
+if [ $? -ne 0 ]; then
----------------
Don't you need to provide the URL to the repository? This seems to assume it's run inside a checked out repository.


================
Comment at: utils/release/merge-request.sh:236
+if [ $? -ne 0 ]; then
+  # Failed to get commit message using svn, try git-svn
+  # Dummy command to force the index to rebuild.
----------------
I think if we include the svn url above, there is no need for this... it should then work even if the user doesn't have an svn repository checked out or updated past the target revision.

It does assume the user has 'svn' installed, but that's required for git-svn too.


================
Comment at: utils/release/merge-request.sh:251
+
+bug_url="https://reviews.llvm.org/rL$revision"
+bug_summary="Merge r$revision into the $stable_version branch${commit_summary}"
----------------
Personally, I prefer this to point to the viewvc, but it's not important.


================
Comment at: utils/release/merge-request.sh:255
+if [ -z ${dryrun} ]; then
+  set -x
+fi
----------------
I assume this is just for your debugging?


https://reviews.llvm.org/D30905