attempt-backport: simplify git steps - #107

@phillipj Think you could still take a quick look at this today?

Wrote down the commands executed to get a better mental map of what's happening, please correct me if I missed something:

Before:

$ git am --abort
$ git reset origin/v7.x-staging --hard
$ git remote update -p
$ git checkout origin/v7.x-staging
$ git clean -fd
$ git reset origin/v7.x-staging --hard
$ curl pr.patch | git am

After:

$ git am --abort
$ git remote update -p
$ git checkout origin/v7.x-staging
$ git clean -fd
$ curl pr.patch | git am

At first glance no doubt two reset shouldn't be necessary. I agree we could try without git reset entirely as you described in #100 (comment).

I do wonder if git clean should be moved above git checkout tho? Sounds like an easy way to ensure the checkout won't be stopped because of something dirty in the repo.

As a side note for later, it would be great to be able to write the expected shell commands executed like the examples above in a set of unit tests. That would allow us to agree on the commands listed in those tests, rather than having to understand the sequence of callbacks.

I was testing with this:

git am --abort
git remote update -p
git checkout upstream/v$1.x-staging
git clean -fdx // @rvagg hinted that I should add -x

curl -L https://github.com/nodejs/node/pull/$2.patch | git am

And wasn't able to find when it would fail... @rvagg suggested I should just try it and I kinda agree we probably won't know otherwise because git is pretty darn complex.

I do wonder if git clean should be moved above git checkout tho? Sounds like an easy way to ensure the checkout won't be stopped because of something dirty in the repo.

Hmmm, I can't think of a case where that would happen?

The purpose of that is ensure a clean slate when checking out older branches that may not ignore certain files. Perhaps that will never happen in the bot environment.

As a side note for later, it would be great to be able to write the expected shell commands executed like the examples above in a set of unit tests. That would allow us to agree on the commands listed in those tests, rather than having to understand the sequence of callbacks.

I recently heard of a thing to test bash scripts, maybe we could also use that? I still think we need to test the scripts but... it's a bit difficult. Trying to wrap my head around nock. :S

The purpose of that is ensure a clean slate when checking out older branches that may not ignore certain files. Perhaps that will never happen in the bot environment.

Okey, that makes sense.

To rephrase my previous comment: might be good to ensure the repo is pristine before perform the checkout. Realised clean isn't sufficient tho, more like git reset --hard && git clean -fdx.

I'm okey with trying the changes in the PR as is and dig more if it still doesn't work as expected.

I still think we need to test the scripts but... it's a bit difficult. Trying to wrap my head around nock. :S

Maybe we should separate the git commands and GitHub interaction more than we currently do. Being able to verify the which git commands executes in a certain order, and which labels that should result in is what's important to verify IMO. The HTTP requests made to the GitHub API is trivial in comparison.