DanielLeens commented on PR #10932:
URL: https://github.com/apache/seatunnel/pull/10932#issuecomment-4525103885
Thanks for the documentation update. I reviewed the latest head
`46b87351053b709297eb52edc8aa26e6525dd103` locally against `upstream/dev`. This
is a docs-only review; I did not run Maven locally.
# What problem this PR solves
- User pain point
The current Kubernetes getting-started pages are mostly Minikube-centric,
so first-time users on managed Kubernetes platforms do not get a concise
checklist for the provider-specific items they need to verify before deploying
SeaTunnel.
- Fix approach
This PR adds managed-Kubernetes notes to the English Helm and Kubernetes
getting-started pages, covering registry access, RBAC, storage, networking,
scheduling, and observability.
- One sentence
The content is useful, but the latest head updates only the English pages
and leaves the existing Chinese counterparts out of sync.
# Runtime / user-facing path I checked
```text
English docs path
-> docs/en/getting-started/kubernetes/helm.md
-> docs/en/getting-started/kubernetes/kubernetes.mdx
-> managed-cluster checklist is visible
Chinese docs path
-> docs/zh/getting-started/kubernetes/helm.md
-> docs/zh/getting-started/kubernetes/kubernetes.mdx
-> still old content, no managed-cluster guidance
```
# Core review
Key findings:
1. This PR does not touch runtime code; it is purely documentation.
2. The added English guidance is directionally helpful.
3. The repo already maintains matching Chinese Kubernetes getting-started
pages.
4. The latest head leaves those Chinese pages unchanged, so the user-facing
docs contract is incomplete.
# Findings
Issue 1: the managed-Kubernetes guidance was added only to `docs/en`, while
the existing `docs/zh` counterparts were left unchanged
- Location: `docs/en/getting-started/kubernetes/helm.md:44`
- Problem description:
This repo already has paired English and Chinese Kubernetes
getting-started pages. The latest head adds new managed-cluster guidance to the
English `helm.md` and `kubernetes.mdx`, but does not sync the same scope to
`docs/zh/getting-started/kubernetes/helm.md` and
`docs/zh/getting-started/kubernetes/kubernetes.mdx`.
- Potential risk:
Chinese readers will miss the exact new guidance this PR is trying to
introduce, so the user-visible documentation contract becomes inconsistent
across languages.
- Best improvement suggestion:
Please sync the managed-Kubernetes notes to the existing Chinese pages as
well, so `docs/en` and `docs/zh` stay aligned.
- Severity: Medium
- Already raised by others: No
# Compatibility
- API/config/defaults/protocol/runtime behavior: no change
- User-facing docs behavior: English and Chinese coverage is currently
inconsistent
# Tests / coverage
- No UT/E2E code changes in this PR, so no test-stability review is needed.
# CI status
- GitHub currently reports:
- `mergeable=MERGEABLE`
- `mergeStateStatus=BLOCKED`
- `Build=ACTION_REQUIRED`
- I checked the check-run output. This is not a code failure; GitHub reports
`Workflow run detection failed` and explicitly suggests:
1. enable GitHub Actions in the fork,
2. push an empty commit to retrigger the workflow,
3. or rebase onto the latest `upstream/dev` and push again if the branch
is stale.
### Conclusion: can merge after fixes
1. Blocking items
- Issue 1: please sync the same managed-Kubernetes guidance to the existing
Chinese docs pages.
2. Suggested but non-blocking follow-up
- No extra non-blocking code suggestions from my side.
Overall, the English content is helpful and worth merging. But for the
current head, I would treat the missing `docs/zh` sync as the main blocker.
Separately, the required Build check also needs to be retriggered with the
GitHub Actions guidance above before merge.
--
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]
