adventures-in / chat_app

A group effort to build a chat app and learn as we go.
18 stars 0 forks source link

Avoid unnecessary CI runs #170

Closed nickmeinhold closed 4 years ago

nickmeinhold commented 4 years ago

Fixes #169

from issue

My thinking is that - given branches pushed onto dev are required to be up to date, there is no need to do a CI run on push. I would appreciate a sanity check and will put up a PR in the meantime.

gaslitbytech commented 4 years ago

I'm going to sit on this and think if a squash + merge could ever break the build. I would think if we used the rebase option there might be such edge cases. The docs seem to have github actions that is configured the way it was. I'm inclined to leave it as it was if there is a remote chance inconsistent builds on push.

nickmeinhold commented 4 years ago

Thanks for your input @tourdownunder - I agree if there is any chance of difference it would be worth doing another check but the point I was making was that I don't think there is any chance. I'm happy to be proven wrong but I don't understand how a branch that is required to be up to date with dev before a push could cause dev to be different to that after the push. If so I'd be keen to understand and appreciate discussing this with you.

The docs seem to have github actions that is configured the way it was.

Are you saying that means we should be doing the same? imo that's just an example and doesn't lend any support to an argument that we should do the same in our particular case.

Thanks for the discussion!

nickmeinhold commented 4 years ago

I did some reading and I feel a bit more confident that in our case, where branches are required to be up to date before merging onto dev, we don't need to have a CI run on a push onto dev.

In Types of required status checks - GitHub Help they state that for the 'loose' type of required status check, where the branch does not have to be up to date with the base branch before merging:

Status checks may fail after you merge your branch if there are incompatible changes with the base branch.

They don't explicitly state that in the "strict" type of required status check the opposite is true but to me that's clearly implied by the omission.

@tourdownunder do you interpret the help-doc that way as well?

nickmeinhold commented 4 years ago

The only other relevant info I've found is the description in Settings where we set the branch to require being up to date:

Screen Shot 2020-06-15 at 10 13 46 pm
gaslitbytech commented 4 years ago

Thanks @nickmeinhold for all your research. I think this setting could make the difference and I'm happy that it is checked.

nickmeinhold commented 4 years ago

Thanks for the discussion @tourdownunder , great to talk it through and make sure it's not a bad idea.