[reland] Add annotations to PRs from forks #54779
Conversation
💊 CI failures summary and remediationsAs of commit 1c310e0 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
|
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| - name: Add annotations | ||
| uses: pytorch/add-annotations-github-action@master | ||
| flake8 | tee ${GITHUB_WORKSPACE}/flake8-output/warnings.txt | ||
| - name: Upload artifact |
There was a problem hiding this comment.
wondering did we measure the impact of these artifacts upload in terms of performance/overall CI time? clang-tidy can get huge
There was a problem hiding this comment.
oh good question; the clang-tidy artifacts uploaded on the workflows for this PR have been only 1.29 KB, but idk what happens when there are actual warnings
Codecov Report
@@ Coverage Diff @@
## master #54779 +/- ##
==========================================
+ Coverage 77.43% 77.45% +0.01%
==========================================
Files 1893 1894 +1
Lines 186424 186403 -21
==========================================
+ Hits 144353 144374 +21
+ Misses 42071 42029 -42 |
|
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
Reverting because apparently this is running on |
|
This pull request has been reverted by ec1bbe1. |
|
@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
samestep
changed the title
Summary: #54779 split out the logic from our "Lint" workflow into a separate workflow that allows us to annotate PRs from forks. However, as of #54689, it is possible for the "Lint" workflow to be canceled, in which case it may not upload the "flake8-py3" and "clang-tidy" artifacts that the "Add annotations" workflow expects. This often results in GitHub pointlessly sending notification emails due to the failure in the "Add annotations" workflow. This PR fixes the issue by gracefully handling the case where the expected artifact is absent. Pull Request resolved: #55242 Test Plan: I tested this in the same external sandbox repo used to test #54779. Reviewed By: malfet Differential Revision: D27540120 Pulled By: samestep fbshipit-source-id: 47cc02950edbbc6381033bda2fe4570cb3e331cb
3 participants
We've been using pytorch/add-annotations-github-action to add annotations to PRs when they fail Flake8 or clang-tidy. Up until now, though, that functionality has only worked on PRs in pytorch/pytorch itself, not on PRs from forks. This PR fixes that using a technique from this GitHub blog post (also linked in a comment in this diff).
Test plan:
@janeyx99 and I tested this in the same GitHub repo used to test #54685 and #54693, including with PRs from forks.