Repository navigation
Conversation
|
I reproduced the empty-report failure on current main ( I tested the PR-overlap shell step at your exact head
All five scenarios behaved as expected on this PR. The four scenarios with report files also passed on main, providing controls for the reproduction. No live comment creation, editing or deletion was performed during the checks. This validates the empty-glob fix and these PR-overlap shell paths. The other workflows and end-to-end GitHub summary presentation were not tested. |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks. Points to address:
- Mixed topics.
shopt -s nullgloband|| trueinpr-overlap.yml(lines 29-30, 38) change behaviour: they fix the empty-report failure Sanjaysrinivas reproduced. That is a bug fix, not observability, and the description doesn't mention it. Please move it to its own PR (it can land quickly) and keep this one logging-only. - No evidence it runs. "Not tested" plus no checks on the branch. Please dispatch
PR overlapandProgresson your fork and link the runs showing the log lines and Job summary. The only local check so far (Sanjaysrinivas's comment) covers thepr-overlapstep, with mocks. Inprogress.yml, thegrep -E '^libraries |^shaders 'does match whattools/progress.pyprints (lines 337-338), butpython3 tools/progress.py out | teeruns withoutpipefail(the defaultrunshell isbash -e), so a failingprogress.pyis only caught by accident when that grep finds nothing. Please addset -o pipefail. - Size vs. value.
progress-comment.ymlgoes from 13 to 56 lines in that step to count six states that are only printed. Please cut it to the stale-run skip line and the unchanged-comment skip, which are the only ones that carry information. - Overlap. #1229 and #1325 also touch
tools/pr_overlap.py; rebase after they land or confirm there is no conflict.
Workflow behaviour changes outside CONTRIBUTING are boykopovar's call, so the above evidence will help him decide.
09ef50d to
1ef9dc6
Compare
|
I reviewed the overlap with Marc's #2134. The changes address separate behavior:
They can coexist and do not depend on each other. Since both touch the report writer, preserve |
3dd7b70 to
6b3000d
Compare
|
Rebased |
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the rebase. Remaining points:
- Description vs diff. The summary says Progress comment gets "the same sync counters as overlap", but
progress-comment.yml:30-46only echoesskipped stale run/unchanged comment. There are no created/updated counters and no job summary. Either add them or fix the description. - Behaviour change.
progress-comment.yml:36-46now skips the PATCH when the body is unchanged. That is a behaviour change, not logging, and the description doesn't list it as one. Please state it under What (it is fine to keep, pr-overlap already does the same). - No evidence for two of the four changes. The linked runs are PR overlap and Progress, on head
1ef9dc62(the Pages deploy step fails on the fork, as expected). Nothing shows theprogress-report.yml:34-41empty-diff summary or theprogress-comment.ymllog lines running. Please link a run of each (a PR on your fork is enough) and re-run Progress on the current head, since thepipefail+grepinprogress.yml:25-35fails the publish step if a line is missing. - Per the earlier comment by @MarcZX100, keep
encoding="utf-8"in the report writer when #2134 lands.
6b3000d to
4973a9c
Compare
|
@adriadam10 Addressed the Oct 11 review on head Description: Progress comment no longer claims overlap-style sync counters; documented the intentional unchanged-body PATCH skip under What. Evidence (fork, same HEAD):
#2134: Please take another look when you have a moment. |
Surface what maintenance and progress workflows did without extra API calls. - PR overlap: scan summary from pr_overlap.py; comment sync counters and job summary - Progress comment: stale-run skip and unchanged-comment skip logging - Progress report: job summary when compare is empty - Progress (Pages): pipefail on progress.py; copy libraries/shaders lines into job summary
4973a9c to
79d90c3
Compare
Summary
Make maintenance and progress GitHub Actions easier to audit: log what each run did and mirror key lines in the Job summary tab, without extra API calls.
pr-overlap.yml,tools/pr_overlap.py): print open-PR scan totals; log comment create/update/delete/unchanged/clean; job summary sync table.progress-comment.yml): log when a Progress report run is stale (PR head moved sinceworkflow_run.head_sha); skip PATCH when the existing bot comment body is unchanged (same pattern as PR overlap); logunchanged comment.progress-report.yml): job summary when compare is empty (No library/shader progress changes in this diff).progress.yml): teeprogress.pyoutput withpipefail; copylibraries/shaderslines into job summary.Test plan
unchanged commentorskipped stale run(not full overlap-style sync counters).workflow_dispatchand confirm log lines + job summary counts.What
CI observability: workflow logs and
$GITHUB_STEP_SUMMARYfor PR overlap, progress report, progress comment, and Pages progress publish.Behaviour (intentional): Progress comment no longer PATCHes GitHub when the composed body matches the existing
github-actions[bot]comment (reduces API churn; mirrors PR overlap unchanged handling).Report files keep explicit UTF-8 encoding from merged #2134 (
write_text(..., encoding="utf-8")).Tested
Head:
4973a9c64865299846e3ebd50d708b3ffd2f0943(rebased onto upstreammain).pr_overlap: repo=... open_prs=...;pr_overlap sync: created=0 ... clean=1; Job summary tablelibraries/shaderslines + Job summary; job fails on forkdeploy-pages(expected)No library/shader progress changes in this diff.progress comment sync: unchanged comment on PR #3(forkmaintemporarily FF'd to run this workflow; restored)Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository (attach them to this PR)