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

Hans Wennborg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 24 04:00:20 PDT 2017


hans added inline comments.


================
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 "
----------------
tstellar wrote:
> hans wrote:
> > 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.
> This is optional, and does map to bugzilla Product field.  The default is to use the new-bugs product, I thought having a project option would be useful if people wanted to be able to categorize their bugs, but I don't mind dropping this and the -component option if it helps simplify the script.
I think we should just drop it.


================
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}"
----------------
tstellar wrote:
> hans wrote:
> > Personally, I prefer this to point to the viewvc, but it's not important.
> Is there a way to get viewvc to show you the full diff with all changes on a single page?
I don't think so, which does make Phabricator seem like a better alternative :-)


================
Comment at: utils/release/merge-request.sh:255
+if [ -z ${dryrun} ]; then
+  set -x
+fi
----------------
tstellar wrote:
> hans wrote:
> > I assume this is just for your debugging?
> Yes, but I think it could be useful for users to verify they've entered the correct options.
Oh right, yes that makes sense.


================
Comment at: utils/release/merge-request.sh:241
+commit_summary=''
+commit_msg=`svn log https://llvm.org/svn/llvm-project/llvm/trunk@$revision`
+if [ $? -ne 0 ]; then
----------------
I think the URL should just be https://llvm.org/svn/llvm-project/
That way it will work for revisions to any module, not just llvm.

Also, I think the current command will print the *whole* log going back from $revision. I think this is the way to do it:
svn log -r $revision http://llvm.org/svn/llvm-project


https://reviews.llvm.org/D30905





More information about the llvm-commits mailing list