DanielCarter-stack commented on PR #10510:
URL: https://github.com/apache/seatunnel/pull/10510#issuecomment-3952504403

   <!-- code-pr-reviewer -->
   <!-- cpr:pr_reply_v2_parts {"group": "apache/seatunnel#10510", "part": 1, 
"total": 1} -->
   ### Issue 1: Missing 3.0.0-specific migration guide and release notes
   
   **Location**: `docs/en/introduction/concepts/incompatible-changes.md`
   
   **Related Context**:
   - Incompatible changes documentation: 
`docs/en/introduction/concepts/incompatible-changes.md`
   - REST API changes (already in dev branch): metrics key format change
   - Flink API refactoring: commit 89b3e1ce1 removed Flink-specific logic
   
   **Issue Description**:
   The current PR only updates the version number but does not create complete 
3.0.0 release notes and migration guide. According to semantic versioning 
specification, a major version upgrade should:
   1. Clearly list all breaking changes
   2. Provide detailed migration guide
   3. Explain upgrade steps and considerations
   
   The current `incompatible-changes.md` documentation only has the `## dev` 
section and does not distinguish specific version numbers. Users cannot clearly 
know which changes belong to 3.0.0.
   
   **Potential Risks**:
   - **Risk 1**: After upgrade, users may experience monitoring failure due to 
REST API metrics key format change
   - **Risk 2**: Flink API refactoring may affect custom connectors or 
extensions
   - **Risk 3**: Transform behavior changes may cause inconsistent data 
processing results
   
   **Impact Scope**:
   - **Direct Impact**: All users planning to upgrade from 2.x to 3.0.0
   - **Indirect Impact**: Third-party monitoring tools that depend on REST API
   - **Impact Area**: Global (all users)
   
   **Severity**: CRITICAL
   
   **Improvement Suggestions**:
   ```markdown
   # Add to incompatible-changes.md:
   
   ## 3.0.0
   
   ### API Changes
   
   - **Breaking Change: Engine REST table metrics key format**
     - (保留现有 dev 章节的内容)
   
   - **Breaking Change: Flink API Refactoring**
     - **Affected component**: Flink connector and translation layer
     - **Description**: Removed Flink-specific logic from SeaTunnel API to 
support multiple parallelisms
     - **Impact**: Custom Flink connectors may need to adapt to the new API
     - **Migration**: Refer to PR #10107 for migration details
   
   ### Transform Changes
   - (保留现有 dev 章节的内容)
   
   ### Migration Guide
   
   1. Update Grafana dashboards to use new metrics key format
   2. Test custom Flink connectors with new API
   3. Verify DataValidator transform output format
   4. Check SQL date/time function results
   ```
   
   **Rationale**: Major version upgrades must have complete migration guides, 
which is the prerequisite for users to upgrade safely.
   
   ---
   
   ### Issue 2: Version number change does not follow semantic versioning 
specification
   
   **Location**: `pom.xml:60`, all documentation and script files
   
   **Related Context**:
   - Last version: 2.3.13 (released in September 2025)
   - This change: 2.3.13-SNAPSHOT → 3.0.0-SNAPSHOT
   - dev branch has 240 commits since 2.3.13 release
   
   **Issue Description**:
   Jumping directly from 2.3.13 to 3.0.0, but:
   1. There is no clear 3.0.0 release plan or design document
   2. Changes after 2.3.13 release are mainly bug fixes (4) and minor 
improvements (1), with only 1 new feature
   3. Although breaking changes exist (REST API, Flink refactoring), whether 
the scale of these changes is sufficient to support a major version upgrade is 
questionable
   4. No RELEASE-NOTES or similar file has been added to explain the 
significance of 3.0.0
   
   **Potential Risks**:
   - **Risk 1**: Overusing major version numbers may reduce user sensitivity to 
truly significant changes
   - **Risk 2**: If 3.0.0 does not actually have enough significant new 
features, users may question the seriousness of the versioning strategy
   - **Risk 3**: If there are larger changes in the future (such as brand new 
architecture), it will face the problem of version number exhaustion
   
   **Impact Scope**:
   - **Direct Impact**: Project versioning strategy and release rhythm
   - **Indirect Impact**: User perception of project maturity
   - **Impact Area**: Project overall
   
   **Severity**: MAJOR
   
   **Improvement Suggestions**:
   1. **If there are indeed significant changes**: Create clear 3.0.0 Release 
Notes documentation, listing all significant changes and new features
   2. **If changes are not significant enough**: Consider using 2.4.0 as the 
new version number (minor version upgrade)
   3. **Regardless of the choice**, clearly explain the rationale for the 
version upgrade in the PR description or related documentation
   
   **Rationale**: Version numbers are important signals for a project and must 
be used carefully. Apache top-level projects should have a clear versioning 
strategy.
   
   ---
   
   ### Issue 3: Documentation example version number updates incomplete
   
   **Location**: Multiple documentation files
   
   **Related Context**:
   - PR changed 15 files
   - Through grep, 21 Markdown files were found containing 2.3.13 or 3.0.0
   - Changed file list basically matches grep results
   
   **Issue Description**:
   Although the PR updated version numbers in most documentation, there may be 
omissions:
   1. Some connector-specific documentation may still reference old versions
   2. README or other getting started documentation not checked
   3. GitHub issue templates or contribution guidelines may not be updated
   
   **Potential Risks**:
   - **Risk 1**: Users following documentation may download the wrong version
   - **Risk 2**: Documentation inconsistency reduces user trust
   
   **Impact Scope**:
   - **Direct Impact**: New users
   - **Indirect Impact**: User support burden
   - **Impact Area**: Low (few documents)
   
   **Severity**: MINOR
   
   **Improvement Suggestions**:
   Execute the following checks before merging:
   ```bash
   # Ensure no references to 2.3.13 are missed (except in places like 
incompatible-changes.md where history needs to be preserved)
   git diff dev...HEAD --name-only | xargs grep -l "2\.3\.13" || echo "No遗漏"
   ```
   
   **Rationale**: Documentation consistency is the foundation of user 
experience.
   
   ---
   
   ### Issue 4: Missing change impact analysis
   
   **Location**: PR description
   
   **Related Context**:
   - PR template requirement: "Does this PR introduce any user-facing change?"
   - Current PR description section is empty
   
   **Issue Description**:
   The PR description completely does not explain:
   1. Why upgrade from 2.x to 3.0.0?
   2. What are the major changes in 3.0.0?
   3. What should users pay attention to when upgrading?
   4. Are there backward compatibility issues?
   
   **Potential Risks**:
   - **Risk 1**: Reviewers cannot judge the rationality of the version upgrade
   - **Risk 2**: Users lack upgrade guidance after merge
   - **Risk 3**: PMC cannot approve the change based on sufficient information
   
   **Impact Scope**:
   - **Direct Impact**: PR review process
   - **Indirect Impact**: Subsequent user upgrade experience
   - **Impact Area**: Project governance
   
   **Severity**: MAJOR
   
   **Improvement Suggestions**:
   Supplement in the PR description:
   ```markdown
   ### Does this PR introduce any user-facing change?
   
   Yes. This is a major version upgrade from 2.3.13 to 3.0.0.
   
   **Previous behavior**: Version 2.3.13 used the following patterns...
   
   **Changes in 3.0.0**:
   - REST API metrics key format changed (breaking)
   - Flink API refactored to support multiple parallelisms
   - Transform behavior changes in DataValidator and SQL functions
   
   **Migration notes**: Users should review the incompatible-changes.md 
document before upgrading.
   ```
   
   **Rationale**: Apache projects require clear explanation of user-visible 
changes.
   
   ---
   
   ### Issue 5: Correctness of known-dependencies.txt update not verified
   
   **Location**: `tools/dependencies/known-dependencies.txt:31-90`
   
   **Related Context**:
   - This file is used for license checking
   - Records known third-party dependencies and their versions
   - Updated version numbers for 7 shade jars
   
   **Issue Description**:
   Although the version number strings are updated correctly:
   1. Not verified whether these shade jars exist or whether names have changed 
in the new version
   2. Not confirmed whether new dependency items need to be added
   3. Not confirmed whether deprecated dependency items need to be removed
   
   **Potential Risks**:
   - **Risk 1**: If shade jar names actually did not change, license checking 
may fail
   - **Risk 2**: If new third-party dependencies are introduced but not added 
to this file, the release process may fail
   
   **Impact Scope**:
   - **Direct Impact**: Build and release process
   - **Indirect Impact**: License compliance
   - **Impact Area**: Build system
   
   **Severity**: MAJOR
   
   **Improvement Suggestions**:
   1. Explain in the PR how the correctness of these dependency items was 
verified
   2. Or execute build testing before merging to confirm license checking passes
   3. Add comments explaining the correspondence between this file and the 
shade plugin configuration in pom.xml
   
   **Rationale**: License compliance is a core requirement for Apache projects 
and must be ensured accurate.
   
   ---


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