Stop appveyor building branches that already in PRs

[pimterry]

This is the last step to fix #241.

Currently, we build for every PR update (building the PR+master commit) and for every branch push. The branch alone isn't really interesting though, and once a PR is open it'll get built by PR triggers anyway. With this (tiny) change, only that PR build happens, rather than effectively duplicating every build job.

Docs for this option are minimal at best, but appveyor/ci#882 has some background.

[pimterry]

For some reason there's two builds on this branch, which I'm not really sure how to interpret. Maybe build notification control is managed by this only once it's on master? ...or maybe it doesn't work?

[Page-]

I expect one build would have been the initial push, before you opened the PR, and the second would be when opening the PR? And only subsequent pushes after opening the PR would be built? I think it's probably worth looking if there's a way to disable building non-master branches altogether unless they are part of a PR, that would avoid any builds of a branch before a PR is opened

[pimterry]

@Page- Interesting, ok. That's definitely possible and really easy if it's preferable. That's actually what Juan suggested initially. I'll update this PR to that and see what happens (I think it should build the resulting PR, but not the branch at all - let's find out!)

[Page-]

Yeah, I don't think we care if branches build or not before it gets PRed (it could be pushing as a WIP thing)

[emirotin]

LGTM. Please rebase before merging (it has pulled two older commits because I did squashes in the sdk-browser branch).

[pimterry]

@jviotti, @emirotin, @Page-: Sadly, I don't think this actually works.

It's now not building this PR for appveyor at all. I think it'll build when we merge to master, but that's obviously not ok. I'm going to go back to the previous version (build all branches on push, but once they're in PRs only build the PR), since it's strictly better than the current status.

I'd be very interested if anybody thinks they can work out how to configure Appveyor to improve beyond this. One other option I've seen discussed is to change which webhooks reach Appveyor in the first place, to only notify it in the first place for PRs, but I can't see the webhook settings for this repo. Who can? Is there an option in there that would let us turn off push notifications and leave on PR notifications? That would be a good fix instead of this, if it exists.

[emirotin]

I'm fine with either solution as long as it builds on PRs :)
I can't see the hooks settings either, it's for Juan to check.

[jviotti]

@pimterry I'll get on it. We have Appveyor priority support, so we'll get this sorted ASAP.

[jviotti]

@pimterry

You can just disable "push" even for AppVeyor's webhook on GitHub web site (repo settings -> Webhooks).

I'll do it myself!

[jviotti]

Done! Let me know if that works!

[Page-]

I'm good with this as an improvement for sure, but if we can find extra ones to add on later that'd be great too :)

[pimterry]

Closing this, as @jviotti's change to the hooks themselves solves the problem better by disabling non-PR branch builds altogether (and seems to work very nicely, judging from #245)