-
Notifications
You must be signed in to change notification settings - Fork 3
Consider all companions from all PRs descriptions #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider all companions from all PRs descriptions #19
Conversation
…m and simplification; pre-merge flows can be implemented on some other check more context given in paritytech#17 (comment)
check_dependent_project.sh
Outdated
|
||
echo "Patching $this_repo into $dependent" | ||
diener patch \ | ||
--target "$org_github_prefix/$comp" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we'll be using --target
from now on instead of the fixed --substrate
, --polkadot
, --cumulus
, etc., $this_repo_diener_arg
can be deprecated. I plan to do so in a future PR (#20).
fi | ||
|
||
echo "running checks for the default branch of $dependent_repo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just duplicated logic which was moved into patch_and_check_dependent
paritytech/substrate#10496 (comment) The implementation scope for this PR is complete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a couple of nits. Otherwise, it should be tested.
@TriplEight it was tested paritytech/substrate#10496 (comment) |
Attempting to merge $repo#$pr_number with master after fetching its last $merge_ancestor_max_depth commits. | ||
|
||
If this step fails, either: | ||
|
||
- $repo#$pr_number has conflicts with master | ||
|
||
*OR* | ||
|
||
- A common merge ancestor could not be found between master and the last $merge_ancestor_max_depth commits of $repo#$pr_number. | ||
|
||
Both cases can be solved by merging master into $repo#$pr_number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
related to #17
trialed in paritytech/substrate#10496
closes #3