Nice! I like it.

Please put this in the tools directory.


http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh
File push-to-trunk.sh (right):

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode30
push-to-trunk.sh:30: TRUNKBRANCH=trunk-push
For safety sake, shouldn't you delete the trunk-push branch every time,
too, just like the prepare-push branch.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode59
push-to-trunk.sh:59: git checkout -b $BRANCHNAME svn/bleeding_edge
You probably want to check that there are no uncommitted changes on the
current branch before switching to another branch.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode111
push-to-trunk.sh:111: $EDITOR src/version.cc
How about suggesting the next logical version number based on an uptick
of the build number and only requiring the editing step if the user
doesn't accept the default?

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode118
push-to-trunk.sh:118: Now working on version
$NEWMAJOR.$NEWMINOR.$NEWBUILD."
Error handling in case something goes wrong?

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode123
push-to-trunk.sh:123: git cl upload -r $REVIEWER --send-mail
You should probably allow the user to specific --prepare-push and
--do-push to do everything up to this point and after this point in two
separate steps.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode128
push-to-trunk.sh:128: read ANSWER
If seems a bit fragile, a stray return proceeds without a LGTM. It would
be neat if you polled the generated CL every few seconds here and only
proceeded if it contained the LGTM.


Also seems really dangerous to do this without error checking the result
of git cl upload.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode133
push-to-trunk.sh:133:
I don't think this does what you want. What it somebody else committed
something to bleeding_edge in the meantime, too? Then that becomes part
of the trunk push patch, which is wrong.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode159
push-to-trunk.sh:159:
Might be a good idea to show the resulting commit message and ask for
verification of the contents.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode164
push-to-trunk.sh:164: patch -p1 < "$PATCHFILE"
Should probably check for errors applying the patch and abort, just in
case.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode177
push-to-trunk.sh:177: git commit -F "$COMMITMSG"
Error handling?

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode188
push-to-trunk.sh:188: git svn dcommit
Error handling, you definitely don't want to make a tag without knowing
the commit worked.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode194
push-to-trunk.sh:194: git checkout -f master
Not everybody has a master (like me). You probably should remember the
current branch from which you started the script and change back to
that, it's more user friendly.

http://codereview.chromium.org/7835035/diff/1/push-to-trunk.sh#newcode201
push-to-trunk.sh:201: echo "Please don't forget to update the v8rel
spreadsheet, and \
For extra points and the respect of the entire team, automatically
update v8rel using http://code.google.com/apis/spreadsheets/ :-)

http://codereview.chromium.org/7835035/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to