Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make check-dependent-* only be executed in PRs #4588

Merged
merged 1 commit into from
Dec 27, 2021

Conversation

joao-paulo-parity
Copy link
Contributor

Make check-dependent-* jobs only be executed in PRs instead of both PRs and master.

Reason 1: The companion is not merged at the same time as the parent PR (1), therefore the pipeline will fail on master since the companion PR is not yet merged in the other repository. This scenario is demonstrated by the pipeline of paritytech/substrate@82cc374.

Reason 2: The job can still fail on master due to a new commit on the companion PR's repository which was merged after bot merge happened, as demonstrated by the following scheme:

  1. Parent PR is merged
  2. Companion PR is updated and set to merge in the future
  3. In the meantime a new commit is merged into the companion PR repository's master branch
  4. The check-dependent-* job runs on master but, due to the new commit, it fails for unrelated reasons

While "Reason 2" can be used as an argument against this PR, in that it would be useful to know if the integration is failing on master, "Reason 1" should be taken care of due to this inherent flaw of the current companion build system design.

Make check-dependent-* jobs only be executed in PRs instead of both PRs and
master.

Reason 1: The companion is not merged at the same time as the parent PR
([1](paritytech/parity-processbot#347 (comment))),
therefore the pipeline will fail on master since the companion PR is not yet
merged in the other repository. This scenario is demonstrated by the pipeline
of
paritytech/substrate@82cc374.

Reason 2: The job can still fail on master due to a new commit on the companion
PR's repository which was merged after `bot merge` happened, as demonstrated by
the following scheme:

1. Parent PR is merged
2. Companion PR is updated and set to merge in the future
3. In the meantime a new commit is merged into the companion PR repository's
  master branch
4. The `check-dependent-*` job runs on master but, due to the new commit, it
  fails for unrelated reasons

While "Reason 2" can be used as an argument against this PR, in that it would
be useful to know if the integration is failing on master, "Reason 1" should be
taken care of due to this inherent flaw of the current companion build system
design.
@joao-paulo-parity joao-paulo-parity added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Dec 23, 2021
@joao-paulo-parity joao-paulo-parity requested a review from a team as a code owner December 23, 2021 08:57
@alvicsam alvicsam removed the request for review from paritytech-ci December 23, 2021 09:54
@ordian ordian merged commit 138535e into paritytech:master Dec 27, 2021
ordian added a commit that referenced this pull request Dec 27, 2021
* master:
  make check-dependent-* only be executed in PRs (#4588)
  session_info: add dispute_period and random_seed (#4547)
  session-info: add new fields + migration (#4545)
  Bump zstd from 0.9.0+zstd.1.5.0 to 0.9.1+zstd.1.5.1 (#4597)
  Relaunch Rococo (#4577)
  Companion for substrate#9732 (#4104)
  Better logs and metrics on PoV fetching. (#4593)
drahnr pushed a commit that referenced this pull request Jan 4, 2022
Make check-dependent-* jobs only be executed in PRs instead of both PRs and
master.

Reason 1: The companion is not merged at the same time as the parent PR
([1](paritytech/parity-processbot#347 (comment))),
therefore the pipeline will fail on master since the companion PR is not yet
merged in the other repository. This scenario is demonstrated by the pipeline
of
paritytech/substrate@82cc374.

Reason 2: The job can still fail on master due to a new commit on the companion
PR's repository which was merged after `bot merge` happened, as demonstrated by
the following scheme:

1. Parent PR is merged
2. Companion PR is updated and set to merge in the future
3. In the meantime a new commit is merged into the companion PR repository's
  master branch
4. The `check-dependent-*` job runs on master but, due to the new commit, it
  fails for unrelated reasons

While "Reason 2" can be used as an argument against this PR, in that it would
be useful to know if the integration is failing on master, "Reason 1" should be
taken care of due to this inherent flaw of the current companion build system
design.
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
Make check-dependent-* jobs only be executed in PRs instead of both PRs and
master.

Reason 1: The companion is not merged at the same time as the parent PR
([1](paritytech/parity-processbot#347 (comment))),
therefore the pipeline will fail on master since the companion PR is not yet
merged in the other repository. This scenario is demonstrated by the pipeline
of
paritytech/substrate@82cc374.

Reason 2: The job can still fail on master due to a new commit on the companion
PR's repository which was merged after `bot merge` happened, as demonstrated by
the following scheme:

1. Parent PR is merged
2. Companion PR is updated and set to merge in the future
3. In the meantime a new commit is merged into the companion PR repository's
  master branch
4. The `check-dependent-*` job runs on master but, due to the new commit, it
  fails for unrelated reasons

While "Reason 2" can be used as an argument against this PR, in that it would
be useful to know if the integration is failing on master, "Reason 1" should be
taken care of due to this inherent flaw of the current companion build system
design.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants