dsuhinin commented on PR #62174: URL: https://github.com/apache/airflow/pull/62174#issuecomment-4096251882
> **Re-review: 2 items still not fixed on the branch** > > The production code (`decorator.py`) and the overall test structure look good. The `injected_for_ordering` approach correctly addresses the original concern about missing required args passing silently. > > From the previous "3 remaining items" review: > > 1. **Test ID `param_after_first_default_without_default`** — Author's rebuttal accepted (`lambda a, b=5, c` is invalid syntax). Just rename the ID to something accurate. > 2. **`list(KNOWN_CONTEXT_KEYS)` → `sorted(KNOWN_CONTEXT_KEYS)`** — Still `list()` on the branch despite the "fixed" reply. Needs the change. > 3. **Docstring vs assertion mismatch in `test_variadic_and_keyword_only_params_are_not_assigned_defaults`** — Still unchanged. Needs either a simplified docstring or an actual assertion on defaults. > > Items 2 and 3 appear to have not made it into the pushed code. Once those two one-liners land, this is good to merge. hey @kaxil . I believe I've fixed all your comments. Please take a look when you will have some time. -- 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]
