DanielCarter-stack commented on PR #10366: URL: https://github.com/apache/seatunnel/pull/10366#issuecomment-3767963636
<!-- code-pr-reviewer --> Thanks! Two MINOR suggestions: - **ExcelGeneratorTest.java:78** - Test uses `sinkColumnsIndexInRow = Arrays.asList(0, 1, 2, 3)` with consecutive indices. With consecutive indices, the old code `createCell(i)` and new code `createCell(index++)` produce identical results (since `i == index`). The bug only manifests with non-consecutive indices (e.g., `[3, 1, 2, 0]`), so the current test cannot verify the fix. Consider adding a test with reordered indices to prevent regression. - **PR Description** - The "Does this PR introduce any user-facing change?" section is empty. Since this fix changes Excel output column order from incorrect (original index order) to correct (`sink_columns` order), users who relied on the previous incorrect order will see different output after upgrade. Documenting this behavior change would help with release notes and user migration. -- 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]
