statechannels / graph-payments

State Channels payment channels managers for paid attestations on The Graph.
MIT License
7 stars 1 forks source link

Consider unique branch names for nightly builds #36

Open geoknee opened 3 years ago

geoknee commented 3 years ago

I'm not entirely sure what will happen when the next nightly build happens, because the GH action is going to try and reuse this branch. We might want to think about unique branch names for the GH action.

Originally posted by @geoknee in https://github.com/statechannels/graph-payments/issues/31#issuecomment-776107004

geoknee commented 3 years ago

Looks like the GH action force pushes the pull request, overwriting it.

I think this is probably undesirable, so we should go ahead with unique branch names.

https://stackoverflow.com/questions/52823692/git-push-force-with-lease-vs-force#:~:text=%2D%2Dforce%2Dwith%2Dlease%20is%20a%20safer%20option%20that%20will,elses%20work%20by%20force%20pushing.

geoknee commented 3 years ago

It's not clear to me if we want to lose commits where nightly builds are failing, which happens when the github action force pushes to an existing branch.

If we only want to tackle failing builds on a relatively low frequency (such as weekly, or just when we have the time), then I am not sure of the benefit of publishing and testing nightly builds?

Let's discuss pros and cons.

andrewgordstewart commented 3 years ago

I prefer using the same branch name, with the existing force-push behaviour. Then, the circleci runs on the "special branch" serve as a paper trail pointing out when it started failing:

Screen Shot 2021-02-11 at 4 25 48 PM
geoknee commented 3 years ago

That's definitely a pro. But it seems a shame to not be able to check out one of those commits in the paper trail?

Perhaps the GH action should actually only run on a actual, long-lived special branch. As in, it should be aiming to update a branch that already contains the most recent nightly branch, rather than master. We can configure CI to run tests on push to this branch, and then we no longer need to raise PRs to get CI to run.

Also, we get to choose to manually raise a PR into master from said special branch, rather than suffering from the noise of a daily PR.

We can also have a slack alert for a failing test on said branch, if we really want to.

Thoughts?

lalexgap commented 3 years ago

Moving to icebox for now as we're not actively working on graph-payments