Changes to review script to support github fork.

Changes to review script to support github fork.

[Joachim Metz, 2015-02-23 11:52:32]

Code updated.

[Joachim Metz, 2015-02-23 12:45:23]

Code updated.

[Joachim Metz, 2015-02-23 12:55:27]

Ready for review.

[onager, 2015-02-28 21:09:09]

I can't actually tell if this for dfvfs or plaso - I assume plaso. If this is
actually for dfvfs, then the references to the git url being wrong are obviously
incorrect =)

Also, perhaps we want to consider using the git credential API:
https://www.kernel.org/pub/software/scm/git/docs/technical/api-credentials.html
now if we're going to be doing these larger changes to the dev process.

https://codereview.appspot.com/198670043/diff/40001/utils/common.sh
File utils/common.sh (right):

https://codereview.appspot.com/198670043/diff/40001/utils/common.sh#newcode10
utils/common.sh:10: GIT_URL="https://github.com/log2timeline/dfvfs.git";
plaso.git

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh
File utils/review.sh (right):

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode85
utils/review.sh:85: echo "             with: Rietveld
(https://codereview.appspot.com)";
no colon needed here

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode103
utils/review.sh:103: echo "Run: 'git remote add upstream
https://github.com/log2timeline/dfvfs.git'";
Change to the plaso repo

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode224
utils/review.sh:224: curl -s --data "${POST_DATA}"
https://api.github.com/repos/log2timeline/dfvfs/pulls?access_token=${ACCESS_T...;
*plaso

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode276
utils/review.sh:276: TEMP_FILE=`mktemp .tmp_dfvfs_code_review.XXXXXX`;
.tmp_plaso_code_review

https://codereview.appspot.com/198670043/diff/40001/utils/submit.sh
File utils/submit.sh (right):

https://codereview.appspot.com/198670043/diff/40001/utils/submit.sh#newcode92
utils/submit.sh:92: echo "Review upload aborted - missing upstream.";
Submit aborted - and also other refs in the file.

[Joachim Metz, 2015-03-05 18:58:51]

The CL is for dfvfs, primarily added you for input on the workflow. The idea is
to port it to plaso.

[Joachim Metz, 2015-03-06 03:15:42]

https://codereview.appspot.com/198670043/diff/40001/utils/common.sh
File utils/common.sh (right):

https://codereview.appspot.com/198670043/diff/40001/utils/common.sh#newcode10
utils/common.sh:10: GIT_URL="https://github.com/log2timeline/dfvfs.git";
On 2015/02/28 21:09:09, onager wrote:
> plaso.git

this is dfvfs

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh
File utils/review.sh (right):

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode85
utils/review.sh:85: echo "             with: Rietveld
(https://codereview.appspot.com)";
On 2015/02/28 21:09:09, onager wrote:
> no colon needed here

I'll leave this as is.

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode103
utils/review.sh:103: echo "Run: 'git remote add upstream
https://github.com/log2timeline/dfvfs.git'";
On 2015/02/28 21:09:09, onager wrote:
> Change to the plaso repo

this is dfvfs

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode224
utils/review.sh:224: curl -s --data "${POST_DATA}"
https://api.github.com/repos/log2timeline/dfvfs/pulls?access_token=${ACCESS_T...;
On 2015/02/28 21:09:09, onager wrote:
> *plaso

this is dfvfs

https://codereview.appspot.com/198670043/diff/40001/utils/review.sh#newcode276
utils/review.sh:276: TEMP_FILE=`mktemp .tmp_dfvfs_code_review.XXXXXX`;
On 2015/02/28 21:09:09, onager wrote:
> .tmp_plaso_code_review

this is dfvfs

https://codereview.appspot.com/198670043/diff/40001/utils/submit.sh
File utils/submit.sh (right):

https://codereview.appspot.com/198670043/diff/40001/utils/submit.sh#newcode92
utils/submit.sh:92: echo "Review upload aborted - missing upstream.";
On 2015/02/28 21:09:09, onager wrote:
> Submit aborted - and also other refs in the file.

Good catch, I will update this in update.sh as well.

[Joachim Metz, 2015-03-06 03:18:43]

Code updated.

[Joachim Metz, 2015-03-06 03:20:02]

https://codereview.appspot.com/198670043/diff/60001/utils/update.sh
File utils/update.sh (right):

https://codereview.appspot.com/198670043/diff/60001/utils/update.sh#newcode104
utils/update.sh:104: git pull upstream master
do we want to run this automatically or manually in case we have merge
conflicts?

[kiddi, 2015-03-14 16:52:26]

one comment

https://codereview.appspot.com/198670043/diff/60001/utils/update.sh
File utils/update.sh (right):

https://codereview.appspot.com/198670043/diff/60001/utils/update.sh#newcode104
utils/update.sh:104: git pull upstream master
On 2015/03/06 03:20:02, Joachim Metz wrote:
> do we want to run this automatically or manually in case we have merge
> conflicts?

well, in the vast majority of the cases there should not be any merge conflicts
and thus we would like this to be done automatically.... can we detect merge
issues and bail out if there are?

I would think that would be the best approach, if no merge issues do this
automatically and all is good, but bail out if there are merge issues and have
the dev fix them all before the update.

[Joachim Metz, 2015-05-14 11:10:26]

https://codereview.appspot.com/198670043/diff/60001/utils/update.sh
File utils/update.sh (right):

https://codereview.appspot.com/198670043/diff/60001/utils/update.sh#newcode104
utils/update.sh:104: git pull upstream master
On 2015/03/14 16:52:26, kiddi wrote:
> On 2015/03/06 03:20:02, Joachim Metz wrote:
> > do we want to run this automatically or manually in case we have merge
> > conflicts?
> 
> well, in the vast majority of the cases there should not be any merge
conflicts
> and thus we would like this to be done automatically.... can we detect merge
> issues and bail out if there are?
> 
> I would think that would be the best approach, if no merge issues do this
> automatically and all is good, but bail out if there are merge issues and have
> the dev fix them all before the update.

Acknowledged.

[Joachim Metz, 2015-05-14 13:24:30]

Code updated.

[Joachim Metz, 2015-05-14 14:24:46]

Code updated.

[kiddi, 2015-05-22 20:00:23]

Few comments here, otherwise looks good

https://codereview.appspot.com/198670043/diff/100001/utils/merge_submit.sh
File utils/merge_submit.sh (right):

https://codereview.appspot.com/198670043/diff/100001/utils/merge_submit.sh#ne...
utils/merge_submit.sh:115: DESCRIPTION=`echo ${CODEREVIEW} | sed
's/^.*"subject":"\(.*\)","created.*$/\1/'`;
do we want to use this or add something like json_xs as a requirement?

$ curl -s https://codereview.appspot.com/api/198670043/ | json_xs
{
   "private" : false,
   "modified" : "2015-05-14 14:24:46.752270",
   "base_url" : "",
   "owner" : "Joachim Metz",
   "owner_email" : "joachim.metz@gmail.com",
   "reviewers" : [
      "kiddi@kiddaland.net",
      "onager@deerpie.com"
   ],
   "issue" : 198670043,
   "closed" : false,
   "created" : "2015-02-23 11:35:30.592240",
   "cc" : [
      "log2timeline-dev@googlegroups.com"
   ],
   "project" : "",
   "subject" : "Changes to review script to support github fork.",
   "description" : "Changes to review script to support github fork.",
   "patchsets" : [
      1,
      20001,
      40001,
      60001,
      100001
   ]
}

makes it easier to parse the description...

And are we sure that the subject attribute is ALWAYS before the created one? If
you get the ordering differently then this sed command will fail...

https://codereview.appspot.com/198670043/diff/100001/utils/review.sh
File utils/review.sh (right):

https://codereview.appspot.com/198670043/diff/100001/utils/review.sh#newcode224
utils/review.sh:224: curl -s --data "${POST_DATA}"
https://api.github.com/repos/log2timeline/dfvfs/pulls?access_token=${ACCESS_T...;
so we are using pull requests

[Joachim Metz, 2015-05-22 20:04:47]

https://codereview.appspot.com/198670043/diff/100001/utils/merge_submit.sh
File utils/merge_submit.sh (right):

https://codereview.appspot.com/198670043/diff/100001/utils/merge_submit.sh#ne...
utils/merge_submit.sh:115: DESCRIPTION=`echo ${CODEREVIEW} | sed
's/^.*"subject":"\(.*\)","created.*$/\1/'`;
json_xs is not available everywhere so I opt not to use it.

> And are we sure that the subject attribute is ALWAYS before the created one? 

No that is why there is:
test "${DESCRIPTION}" = "${CODEREVIEW}"

https://codereview.appspot.com/198670043/diff/100001/utils/review.sh
File utils/review.sh (right):

https://codereview.appspot.com/198670043/diff/100001/utils/review.sh#newcode224
utils/review.sh:224: curl -s --data "${POST_DATA}"
https://api.github.com/repos/log2timeline/dfvfs/pulls?access_token=${ACCESS_T...;
On 2015/05/22 20:00:23, kiddi wrote:
> so we are using pull requests

yes, since that will trigger travis and appveyor

[kiddi, 2015-05-22 20:13:22]

LGTM