Skip to content

Comments

Initial Project Feedback#6

Open
jdaugherty wants to merge 27 commits intomainfrom
feedback
Open

Initial Project Feedback#6
jdaugherty wants to merge 27 commits intomainfrom
feedback

Conversation

@jdaugherty
Copy link
Contributor

@jdaugherty jdaugherty commented Feb 18, 2026

@jamesfredley & @matrei I've implemented all of the initial feedback that I can. Here are my notes of what differed:

  • @matrei Application class appears to be a requirement in the plugin source: it exceptions in the integration test apps otherwise
  • @matrei the minimum grails version is set to 7.0.7 because when we generate a 7.x app, it chooses the latest Grails version
  • @matrei I intentionally made the TimingMetrics & Metrics public so they could be accessed if desired; so I'd rather model the implementation case instead of runtimeOnly case.
  • @matrei to get the build to work better with windows, i just special cased the theme (so I can have a pretty theme on macs); alternatively, someone has to setup a unicode capable terminal on windows
  • @matrei the autoconfiguration / properties I reverted because of Plugin configuration is loaded after @Conditional checks in Spring apache/grails-core#15408 (i've confirmed the open PR does resolve this and we can revert c0eb2d0 when it's fixed)
  • @jamesfredley the ci#publish & release jobs use different secrets due to how maven central is configured for snapshots. This is intentional.

@jdaugherty
feedback: Server-Timing -> Server Timing where possible
99b3b75
@jdaugherty
feedback: servertiming -> serverTiming for config key
38097a6
@jdaugherty
feedback: Remove unnecessary Application class in plugin
85f93a2
@jdaugherty
Revert "feedback: Remove unnecessary Application class in plugin"
9ab3579 
This reverts commit 85f93a2.
@jdaugherty
feedback: simplify plugin configuration
ee15067
@jdaugherty
feedback: move log configuration to test configuration only
6cf6dde
@jdaugherty
feedback: switch to CompileStatic when GrailsCompileStatic is not needed
d0d3055
@jdaugherty
feedback: rename plugin class
1d283e5
@jdaugherty
feedback - 7.0.7 -> 7.x
2a00b3b
@jdaugherty
feedback: rename title
0e1b0b6
@jdaugherty
feedback: lazy log messages
9fde912
@jdaugherty
feedback: use def for local variables where type is inferred
39effda
@jdaugherty
feedback: prefer extensions.configure() instead of extensionName { }
3a49e8c
@jdaugherty
feedback: use dependsOn() instead of setter
ad3d84e
@jdaugherty
feedback: removing explicit types & it where possible
e1af184
@jdaugherty
feedback: fix display on windows due to unicode usage
6454deb
@jdaugherty
feedback: add auto configuration instead of using the grails plugin d…
471894a 
…escriptor
@jdaugherty
feedback: use ubuntu-24.04 instead of ubuntu-latest
0ec228b
@jdaugherty
feedback: use plugin configuration instead of helper
146a21a
@jdaugherty
revert feedback: remove autoconfiguration & properties until apache/g…
c0eb2d0 
…rails-core#15408 is fixed
@jdaugherty
feedback: handle distribution between release & ci the same
575372c
@jdaugherty
feedback: use gradle properties for versions
6b4fe33
@jdaugherty
feedback: getWriter() and getOutputstream() need to function similarly
9eb0171
@jdaugherty
feedback: fix gradle wrapper updates
90237a8
jamesfredley
jamesfredley approved these changes Feb 19, 2026
View reviewed changes
@jdaugherty
Copy link
Contributor Author

jdaugherty commented Feb 22, 2026

@matrei if you have time to review this, I can get the grails-core docs updated with this as an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamesfredley jamesfredley jamesfredley approved these changes

@matrei matrei Awaiting requested review from matrei

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jdaugherty @jamesfredley