Skip to content

Conversation

atalman
Copy link
Contributor

@atalman atalman commented Dec 20, 2023

Observing following PR: #115329
Comment from author: #115329 (comment)

pytorchbot merge failed.
Reason is this logic, we expect all files in PR to match one merge rule:

for fname in changed_files:
if not patterns_re.match(fname):
non_matching_files.append(fname)
if len(non_matching_files) > 0:
num_matching_files = len(changed_files) - len(non_matching_files)
if num_matching_files > reject_reason_score:
reject_reason_score = num_matching_files
reject_reason = "\n".join(
(
f"Not all files match rule `{rule_name}`.",
f"{num_matching_files} files matched, but there are still non-matching files:",
f"{','.join(non_matching_files[:5])}{', ...' if len(non_matching_files) > 5 else ''}",
)
)
continue

This should mitigate the issue, followup will post a PR to refactor this code to allow cross rule matching of approvers

@atalman atalman requested review from huydhn and jgong5 December 20, 2023 00:12
@atalman atalman requested a review from a team as a code owner December 20, 2023 00:12
@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Dec 20, 2023
Copy link

pytorch-bot bot commented Dec 20, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/116145

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (4 Unrelated Failures)

As of commit 559074f with merge base f88c9af (image):

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Why do we need 3 different rules? For x86 CPU quantization as well as for CPU frontend reviewers are the same, should those two rules be folded together?

@jgong5
Copy link
Collaborator

jgong5 commented Dec 20, 2023

Q: Why do we need 3 different rules? For x86 CPU quantization as well as for CPU frontend reviewers are the same, should those two rules be folded together?

This is because the rules are organized by the modules and the reviewers happen to be the same.

@atalman
Copy link
Contributor Author

atalman commented Dec 20, 2023

@pytorchmergebot merge -f "All required jobs are passing"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

dmenig pushed a commit to dmenig/pytorch that referenced this pull request Dec 21, 2023
…e rule (pytorch#116145)

Observing following PR: pytorch#115329
Comment from author: pytorch#115329 (comment)

pytorchbot merge failed.
Reason is this logic, we expect all files in PR to match one merge rule:
https://github.com/pytorch/pytorch/blob/110339a310e5c6bc0aef04b6b1d848976d10013e/.github/scripts/trymerge.py#L1310-L1324

This should mitigate the issue, followup will post a PR to refactor this code to allow cross rule matching of approvers
Pull Request resolved: pytorch#116145
Approved by: https://github.com/huydhn, https://github.com/kit1980, https://github.com/malfet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants