Replace tools/myvn with new Git workflow helper. (issue 285550043 by kpreid@google.com)

7 views
Skip to first unread message

re...@codereview-hr.appspotmail.com

unread,
Feb 19, 2016, 6:21:42 PM2/19/16
to eri...@gmail.com, kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: MarkM,

Description:
'tools/myvn change' is now 'tools/workflow prepare'.
'tools/myvn snapshot' is now 'tools/workflow snapshot'.
'tools/myvn eclipse' is now 'tools/eclipse-setup'.
Other stuff does not map as well.

Please review this at https://codereview.appspot.com/285550043/

Affected files (+607, -826 lines):
D tools/appspot.py
A tools/eclipse-setup
A tools/log_helper.py
D tools/myvn
A tools/workflow


eri...@gmail.com

unread,
Feb 19, 2016, 8:56:35 PM2/19/16
to kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/285550043/diff/1/tools/eclipse-setup
File tools/eclipse-setup (right):

https://codereview.appspot.com/285550043/diff/1/tools/eclipse-setup#newcode6
tools/eclipse-setup:6: # (5) From Existing Source
On Eclipse Mars, I did not see a "From Existing Sources" option.
Instead I found

Choose File >> New >> Project...

In the resulting dialog box, I opened the Java folder and select
Java Project from Existing Ant Buildfile
Is this what you intended?

The was a checkbox for "Link to the buildfile in the file system."
I am checking this. We'll see how it goes.

https://codereview.appspot.com/285550043/diff/1/tools/eclipse-setup#newcode7
tools/eclipse-setup:7: # (6) Enter the
.../svn-changes/my-change-1/google-caja/ path
No longer svn

https://codereview.appspot.com/285550043/diff/1/tools/eclipse-setup#newcode10
tools/eclipse-setup:10: # $ ant
Well, the option I chose above was probably wrong after all. "ant" ran
successfully, but Eclipse is reporting 3608 static errors.

https://codereview.appspot.com/285550043/diff/1/tools/eclipse-setup#newcode23
tools/eclipse-setup:23: echo " <classpathentry
excluding=\"**/*~|**/.svn/**|nu/validator/htmlparser/dom/|
nu/validator/htmlparser/extra/|nu/validator/htmlparser/io/|
nu/validator/htmlparser/sax/|nu/validator/htmlparser/xom/\"
kind=\"src\" path=\"third_party/java/htmlparser/src\"/>"
Please reformat to a narrower width. Ideally 80 columns, but please no
more than 100.

https://codereview.appspot.com/285550043/

re...@codereview-hr.appspotmail.com

unread,
Feb 22, 2016, 5:10:39 PM2/22/16
to eri...@gmail.com, kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
'tools/myvn change' is now 'tools/workflow prepare'.
'tools/myvn snapshot' is now 'tools/workflow snapshot'.
'tools/myvn eclipse' is now 'tools/eclipse-setup'.
Other stuff does not map as well.


Fix snapshot-when-not-squashed bug.


https://codereview.appspot.com/285550043/

eri...@gmail.com

unread,
Feb 22, 2016, 5:35:21 PM2/22/16
to kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
On 2016/02/22 22:10:37, kpreid_google wrote:
> Fix snapshot-when-not-squashed bug.

I successfully snapshotted. Thanks.

https://codereview.appspot.com/285550043/

re...@codereview-hr.appspotmail.com

unread,
Feb 22, 2016, 6:38:59 PM2/22/16
to eri...@gmail.com, kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
'tools/myvn change' is now 'tools/workflow prepare'.
'tools/myvn snapshot' is now 'tools/workflow snapshot'.
'tools/myvn eclipse' is now 'tools/eclipse-setup'.
Other stuff does not map as well.
Update eclipse instructions.


https://codereview.appspot.com/285550043/

kpr...@google.com

unread,
Feb 22, 2016, 6:39:06 PM2/22/16
to eri...@gmail.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
On 2016/02/20 01:56:30, MarkM wrote:
> On Eclipse Mars, I did not see a "From Existing Sources" option.

These instructions were ancient (and I did not finish updating them).
Updated the whole thing.

https://codereview.appspot.com/285550043/diff/1/tools/eclipse-setup#newcode23
tools/eclipse-setup:23: echo " <classpathentry
excluding=\"**/*~|**/.svn/**|nu/validator/htmlparser/dom/|
nu/validator/htmlparser/extra/|nu/validator/htmlparser/io/|
nu/validator/htmlparser/sax/|nu/validator/htmlparser/xom/\"
kind=\"src\" path=\"third_party/java/htmlparser/src\"/>"
On 2016/02/20 01:56:30, MarkM wrote:
> Please reformat to a narrower width. Ideally 80 columns, but please no
more than
> 100.

This is moved, not new code — it's only in a new file because I split
tools/myvn's job into tools/workflow and this file. I'd rather not
rework and test it as part of this change, since introducing line breaks
is nontrivial due to quoting.

https://codereview.appspot.com/285550043/

eri...@gmail.com

unread,
Feb 22, 2016, 10:24:46 PM2/22/16
to kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/285550043/diff/40001/tools/workflow
File tools/workflow (right):

https://codereview.appspot.com/285550043/diff/40001/tools/workflow#newcode40
tools/workflow:40: # $ tools/workflow validate-change
Since "ant runtests" doesn't actually work at the moment, we should say
something about how to cope.

https://codereview.appspot.com/285550043/diff/40001/tools/workflow#newcode44
tools/workflow:44: # $ git merge --ff-only <my-branch-name>
When I got here, I could not remember what branch name I used. I
remembered "git branch" and look for the star. But this happened after
"git checkout master". You should explain how to get your branch name.

https://codereview.appspot.com/285550043/diff/40001/tools/workflow#newcode115
tools/workflow:115: private-snapshot )
Assuming private snapshots are not ready yet, you should mention in the
instructions that they are coming.

https://codereview.appspot.com/285550043/diff/40001/tools/workflow#newcode122
tools/workflow:122: checkReviewersHappyAndMatchesReview
no such command was found

https://codereview.appspot.com/285550043/diff/40001/tools/workflow#newcode123
tools/workflow:123: if ! "$tools_dir/log_helper.py" fail-if-private
"$@"; then
no fail-if-private was found. This if concluded that we had a private cl
and gave us the message below.

https://codereview.appspot.com/285550043/diff/40001/tools/workflow#newcode141
tools/workflow:141: echo TODO IMPLEMENT
Is the reason this isn't implemented is that you have to fork the "I am
really pushing" vs "I am posting a pull request"? Perhaps these should
be two separate commands anyway?

Since "tools/workflow push" doesn't work yet, for those who can "git
push" you should mention that in the workflow text above. Probably the
same for a pull request, but I don't know.

https://codereview.appspot.com/285550043/diff/40001/tools/workflow#newcode178
tools/workflow:178: else
little bits of extra whitespace here and there.

https://codereview.appspot.com/285550043/
Reply all
Reply to author
Forward
0 new messages