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

   @sg-c0de 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: **Missing Tests**: The PR adds new Helm chart behavior (Celery 
worker image overrides via new helper templates and new values) but no Helm 
unit test files are included in the changeset. Per review guidelines, new 
public behavior requires tests covering success, failure, and edge cases. The 
Helm tests suite has a dedicated area for exactly this kind of templating logic.
   - :warning: **Description Does Not Match Code**: The PR description states 
the new values are at `Values.workers.celery.image` and 
`Values.workers.celery.sets[].image`, but the `_helpers.yaml` patch references 
`.Values.workers.image` (not `.Values.workers.celery.image`). The values.yaml 
patch adds the `image` block inside the `workers.celery` section (after `queue: 
"default"`). This discrepancy between the described API surface and the actual 
template code is a code-correctness concern that warrants scrutiny from 
maintainers.
   
   > **Note:** Your branch is **73 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