Merge Substrate master before checking Polkadot companion

[bkchr]

No description provided.

[andresilva]

If we're going to do this for substrate should we also do it below for the polkadot branch?

[gabreal]

IMO this will make the pull request test run on different code (most recent master merged in) with what the pull requests' code is currently at. This can lead to inconsistent results and also deliver false results. I would rather suggest to force pr to be actually up to date with the master branch or have a bot push to the branches.

[bkchr]

The point is, we already have failing prs now because the branches are not up-to-date. While you are right that it may not solve all the problems, I think that it at least improves the current situation. People don't understand why their pr does not build with Polkadot, when they only changed internals in Substrate.

[andresilva]

@gabreal I kind of agree that we could instead force PRs to be up-to-date with master, ideally this could be done automatically by a bot. But this also brings other problems with commits going automatically (and maybe unexpectedly) into your PRs, I think for some people this would cause issues when they try to push before pulling the latest changes. The other drawback of the bot approach is that it doesn't exist yet and will take longer to implement than the simple changes in this PR.

I think the changes here will not bring any false positives since ultimately we will merge the PR with master and we want it to be green there. What could cause an issue is when this script tries to merge master and it has conflicts, but since this is only done for the companion task I think it's manageable.

[gabreal]

when fetching with --depth 1 the master branch won't be available in this repo. this will require something like this before.

[gabreal]

(this is the case for all merges here, since GIT_DEPTH is set to only do shallow cloning)

[gabreal]

also the merge of this pr failed:

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'root@runner-m1hcVmhZ-project-145-concurrent-0.(none)')

so for this a git config will be required and potentially some error checking (if desired) if the merge failed.
apart from that i under stand your points and even though i would prefer the other solution the argument that it will be merged with master in the end also makes sense to me. should there be stable branch in the future this may fall on our toes.

[bkchr]

We can remove these merges in the future again, that isn't that complicated :D

Regarding the email & user name, I pushed some fix that hopefully fixes it.

[gabreal]

alright. but still the master branch should be fetched like mentioned in the comment above. why is test-linux-stable failing now?