[GHF] Better check for internal diffs #104344
Conversation
During revert, use title of "Meta Internal-Only Changes Check" to determine whether or not internal diff is associated with the PR. Fixes #104232 [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/104344
Note: Links to docs will display an error until the docs builds have been completed. ✅ 1 Unrelated FailureAs of commit fb87b85: UNSTABLE - The following job failed but was 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. |
During revert, use title of "Meta Internal-Only Changes Check" to determine whether or not internal diff is associated with the PR. Fixes #104232 cc albanD [ghstack-poisoned]
During revert, use title of "Meta Internal-Only Changes Check" to determine whether or not internal diff is associated with the PR. Fixes #104232 cc albanD [ghstack-poisoned]
| if self.get_diff_revision() is None: | ||
| return False | ||
| checks = self.get_checkrun_conclusions() | ||
| if checks is None or checkrun_name not in checks: | ||
| return False | ||
| return checks[checkrun_name].status != "SUCCESS" | ||
|
|
||
| def has_no_connected_diff(self) -> bool: |
There was a problem hiding this comment.
Returning a negative of a boolean value reduces readability, IMO replace this with has_connected_diff
There was a problem hiding this comment.
Sure, but is it safe to say, that has_connect_diff() is true if status is anything but "There is no internal Diff connected, this can be merged now"?
During revert, use title of "Meta Internal-Only Changes Check" to determine whether or not internal diff is associated with the PR. Fixes #104232 cc albanD [ghstack-poisoned]
@bigfootjon I've not change any of the existing APIs nor added new dependencies, so imo it should be fine, but just to be on the safe side: https://www.internalfb.com/intern/sandcastle/job/18014399493129784/ |
|
@pytorchbot merge -f "Lint and relevant internal tests are green" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Stack from ghstack (oldest at bottom):
During revert, use title of "Meta Internal-Only Changes Check" to determine whether or not internal diff is associated with the PR. When PR is merged/closed, "Meta Internal-Only Changes Check" status is always success, but title message can differ:
Add regression test for #100652 that was originated from the internal diff, but was merged as OSS PR.
Fixes #104232
cc @albanD