potiuk commented on PR #63625:
URL: https://github.com/apache/airflow/pull/63625#issuecomment-4065552307

   @nhuantho This PR has a few issues that need to be addressed before it can 
be reviewed — please see our [Pull Request quality 
criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   
   **Issues found:**
   - :warning: **Code Quality — Incorrect Comment**: In the `downgrade()` 
function, the comment reads `# Revert the value column back to LargeBinary`, 
but the code actually reverts `external_executor_id` to `VARCHAR(250)`, not 
`LargeBinary`. This is a factually wrong narrating comment that will mislead 
future readers.
   - :warning: **Testing — Missing Regression Test**: This PR fixes a bug in 
the `downgrade()` path of migration 0102 (batch_alter_table fails on 
PostgreSQL). The description shows only manual screenshot-based verification. 
No automated regression test is added to confirm the downgrade works correctly 
on PostgreSQL and that the original bug does not regress. Airflow has a 
migration-testing infrastructure that should be used.
   - :warning: **Meaningful Description**: The PR body lacks a written 
explanation of *why* `batch_alter_table` fails on PostgreSQL for this migration 
and why the raw `ALTER TABLE … USING CASE` SQL is the correct fix. The 
referenced issue #63545 provides context, but the PR itself should summarise 
the root cause for reviewers without requiring them to navigate away.
   
   > **Note:** Your branch is **17 commits behind `main`**. Some check failures 
may be caused by changes in the base branch rather than by your PR. Please 
rebase your branch and push again to get up-to-date CI results.
   
   **What to do next:**
   - The comment informs you what you need to do.
   - Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - 
but only after making sure that all the issues are fixed.
   - There is no rush — take your time and work at your own pace. We appreciate 
your contribution and are happy to wait for updates.
   - Maintainers will then proceed with a normal review.
   
   There is no rush — take your time and work at your own pace. We appreciate 
your contribution and are happy to wait for updates. If you have questions, 
feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).


-- 
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]

Reply via email to