matrei commented on PR #415:
URL:
https://github.com/apache/grails-intellij-plugin/pull/415#issuecomment-5632726072
## AI Review
Reviewed both commits against `origin/main`, checked every replaced API
against the 2026.2.2 jars, and built the plugin and ran the five new test
classes locally (all 25 pass, see bottom).
The deprecation work is sound: every API retired here is deprecated or
`@ApiStatus.Internal` in 2026.2.2, and every replacement exists with the
signature used. Two things need to change before merge; the rest are follow-ups.
### Should fix before merge
**1. `.asf.yaml` drops `features: issues: true`**
`main` added this flag deliberately on 2026-07-20 (529d03f "Enable GitHub
issues"), and the repo now has ~370 open issues. The "Project clean-up" commit
removes it with no explanation, and the ASF `.asf.yaml` docs don't say what
happens when the key is omitted. This looks like an accidental revert; please
restore it unless turning Issues off is intended.
**2. `GradleSettingsFile` "already included" check is a substring test**
`plugin/src/main/java/org/apache/grails/intellij/plugin/config/GradleSettingsFile.java:247`
```java
if (existing.contains("'" + moduleName + "'") || existing.contains("\"" +
moduleName + "\"")) return;
```
Any quoted occurrence of the module name (a comment, an `includeBuild`, an
unrelated literal) suppresses the include. Agree with the earlier comment on
this thread: detect an actual `include` statement for the module, and check the
`projectDir` mapping separately when the directory differs from the name.
Also note the behaviour change versus the platform call it replaces: for a
fresh project with an existing `settings.gradle`, the old code re-rendered the
file, the new code leaves it alone and only appends the include. The javadoc
documents this so I assume it is intentional; just flagging it.
### Follow-ups (non-blocking)
- **JSP is now a Marketplace dependency.** The 2026.2.2 distribution ships
no JSP plugin, so users must install it. Neither `README.md` nor the plugin
description mentions this. Marketplace normally prompts for required plugins,
but a line in the README "To run" section would avoid confusion.
- **`GrailsBackgroundService` can wedge.** If `ProgressManager.run` throws
synchronously inside `startNext()` (e.g. after project disposal), `myRunning`
stays `true` and nothing queued afterwards ever starts. Low impact since the
service is per project. A `catch` that resets the flag and rethrows would cover
it; agree with the earlier note that a `finally` would be wrong for the async
case.
- **`GrailsAttachSourcesProvider` no longer refreshes `$GRAILS_HOME/src`.**
Necessary now that discovery runs in a background read action, but a sources
jar downloaded outside the IDE will not be seen until a VFS refresh.
Acceptable; a comment would help the next reader.
- **New-file menu weighting is gone.** `WeighingNewActionGroup` is internal
so removing it is right, but the Grails "New" actions will no longer float to
the top of the menu. User-visible, worth a changelog line.
- **`GrailsApplicationManager.findApplication`** now calls
`LocalFileSystem.findFileByPath` on every invocation to find the project
directory. Fine, but `ProjectUtil.guessProjectDir(project)` is the idiomatic
replacement for `getBaseDir()`.
### Verified
- Deprecated in 2026.2.2: `ReadAction.compute`,
`PositionManager.getAcceptedFileTypes`,
`LanguageFileType.isJVMDebuggingSupported`, `BackgroundTaskQueue`,
`Project.getBaseDir`, `DumbService.runReadActionInSmartMode`,
`AttachSourcesProvider.getActions`.
- `@ApiStatus.Internal`: `StartupManager.runAfterOpened`,
`WeighingNewActionGroup`, `MavenExternalParameters.createJavaParameters`,
`AbstractGradleModuleBuilder.setupGradleSettingsFile`.
- `DumbService.runWhenSmart` delegates to `SmartModeScheduler`, which waits
for project startup, so the `MvcModuleStructureSynchronizer` and
`GrailsModuleBuilder` changes keep their pre-open semantics.
- The comment in `GrailsSpringStereotypeAliasingPsiTargetMapper` is
accurate: the old code nested `runReadActionInSmartMode` inside a read action,
so it already threw in dumb mode rather than waiting.
- The `setupEnvs` replacement in `GrailsInstallationCommandExecutor`
reproduces the platform method exactly.
- `pluginSinceBuild=262.10315.125` is required by the JSP unbundling;
`README.md`, `AGENTS.md`, `INSTALL` and `RELEASE.md` were updated consistently.
- `MavenRunConfiguration.createJavaParameters(Project)` is public and not
deprecated.
### Local test run
`./gradlew :plugin:test` restricted to the new classes, JDK 25 / Gradle
9.7.1 per `.sdkmanrc`:
| Test class | Tests | Result |
| --- | --- | --- |
| `MavenCommandExecutorTest` | 5 | pass |
| `GradleSettingsFileTest` | 6 | pass |
| `GrailsAttachSourcesProviderTest` | 10 | pass |
| `GspJavaDebugAwareTest` | 2 | pass |
| `GrailsBackgroundServiceTest` | 2 | pass |
--
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]
