potiuk commented on issue #70296: URL: https://github.com/apache/airflow/issues/70296#issuecomment-5130442279
### Review sweep, 30 July — 15 burn-down PRs I worked through the open burn-down PRs today. Posting the outcome here rather than scattering it across fifteen threads, plus two process observations. **Merged (1):** #70533 (`GCSListObjectsOperator` delimiter) **Approved, awaiting merge (6):** #70449, #70493, #70528, #70529, #70531, #70542 — all genuine value reads, correctly moved to `execute()`. **Changes requested (4):** #70347 (removes `task_id` defaulting — a behaviour change needing its own PR or a newsfragment), plus #70530, #70574, #70723 — see the correction below. **Closed as duplicates (4):** #70621 (dup of #70449), #70678 (dup of #70493), #70437 (dup of #70369), #70656 (dup of #70347). In each case both PRs made substantially the same change to the same files, 1–6 days apart. --- #### I got the provision-check distinction wrong three times Worth recording, because it says something about how easy this is to miss. I approved #70530, #70574 and #70723 before reading this issue properly. All three move a **provision** check into `execute()` — precisely what the "False positives" section says not to do — and I have since retracted those approvals and requested the in-place `is not None` rewrite instead. If a reviewer working through these PRs deliberately can make that mistake three times in one sitting, contributors picking up a "good first issue" entry will too. Two suggestions: - The `validate-operators-init` failure message could name the distinction and link the False positives section directly — that's the moment someone is actually looking. - The five entries listed as "clear with no code change" are easy to miss in a long issue body. It might help to move the "provision check → rewrite in place, don't move" rule above the exemption-ratchet section, since it changes what you do before you start. #### Four contributors collided this week The four duplicates above weren't near-misses — they were the same operator, opened days apart by people who had no way of knowing someone else had started. That's four wasted contributor efforts and four extra review passes. The issue asks people not to open sub-issues, which is sensible, but there's currently no way to see what's taken. A checklist in this issue body of the remaining 23 entries, with contributors ticking or commenting to claim one, would cost little and make in-flight work visible. Alternatively a convention of commenting "taking `<path>::<Class>`" before opening the PR. **Status:** 23 entries remain in `validate_operators_init_exemptions.txt`; 20 open PRs reference this issue. #70505 merged on 28 July, so provision checks written with `is not None` no longer trip the hook — the in-place rewrites above pass and still clear their exemption entries. --- Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
