Aias00 commented on PR #7673: URL: https://github.com/apache/shenyu/pull/7673#issuecomment-6282516329
## Re-review: both previous blocking items are resolved, but `pr_build` is now red Thanks for the rework. I verified the two items from my previous review against head `c9f142aa` and both are genuinely fixed: 1. **`loged` → `logged`** — the parser, `RuleData`, `SelectorData`, the admin DTO/DO/VO layer, the MyBatis maps, the H2/MySQL/Oracle/PG schemas and all five `2.7.2-upgrade-2.7.3-*.sql` scripts are now consistent. Good. 2. **`ApolloDataServiceTest` collision with #7680** — the duplicate fix is gone from the diff. Correct call. 3. The module relocation into `shenyu-spring-boot-starter-sync-data-center/shenyu-spring-boot-starter-sync-data-file` is wired up correctly: parent `<module>`, `spring.factories` for the `EnvironmentPostProcessor`, `AutoConfiguration.imports` for the configuration, the `shenyu-bootstrap` dependency and the commented `application.yml` example all line up. ## 🔴 Blocking — the rename is incomplete, so `pr_build` fails to compile `pr_build` (run [39357244520](https://github.com/apache/shenyu/actions/runs/39357244520)) fails at `testCompile` of `shenyu-plugin-key-auth`: ``` [ERROR] KeyAuthAuthenticationHttpTest.java:[151,17] cannot find symbol [ERROR] symbol: method setLoged(boolean) [ERROR] location: variable rule of type org.apache.shenyu.common.dto.RuleData [ERROR] KeyAuthAuthenticationTest.java:[311,13] cannot find symbol [ERROR] symbol: method setLoged(boolean) ``` `RuleData.setLoged(...)` no longer exists after this PR, but these call sites were not updated. I cross-checked every remaining `loged` occurrence in the tree against your changed-file list, and exactly three files were missed: | File | Line | Current code | |---|---|---| | `shenyu-plugin/shenyu-plugin-security/shenyu-plugin-key-auth/src/test/java/org/apache/shenyu/plugin/key/auth/KeyAuthAuthenticationHttpTest.java` | 151 | `rule.setLoged(false);` | | `shenyu-plugin/shenyu-plugin-security/shenyu-plugin-key-auth/src/test/java/org/apache/shenyu/plugin/key/auth/KeyAuthAuthenticationTest.java` | 311 | `rule.setLoged(false);` | | `shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/test/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/KeyAuthMcpTestFixture.java` | 310 | `.loged(false)` on the `RuleData` builder | The build stops at `shenyu-plugin-key-auth`, so the third one has not surfaced in CI yet but will fail the same way once the first two are fixed. Please update all three (`setLoged` → `setLogged`, builder `loged` → `logged`) and push so `pr_build` can go green. ## ⚠️ Please coordinate the merge order with #7677 `#7677` ("rename misspelled `loged` column/field to `logged`") is still open and currently `CONFLICTING`. This PR now carries the same rename, so the two will collide in `RuleDO`/`SelectorDO`/`RuleDTO`/`SelectorDTO`/`RuleData`, the MyBatis maps and the SQL scripts. One of them should land first and the other should drop its copy — please agree on the order on the relevant issue rather than merging both. If `#7677` lands first, this PR only needs to drop the rename hunks; if this PR lands first, `#7677` becomes redundant and should be closed. Nothing else I reviewed is blocking: `FileModeEnvironmentPostProcessor`, `FileSyncDataConfiguration` and `FileConfigurationLoader` are unchanged in substance from the version I looked at before and still hold up. -- 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]
