Build restructuring#9
Conversation
jdaugherty
left a comment
There was a problem hiding this comment.
Here's what I like:
- general gradle clean up & consistency
- removal of the project specific name so it can be generic (@sbglasius feedback)
- using the type specific extensions like AssetPipelineExtension
- using withPlugin() to reduce configuration
Here's what I think we can improve:
- it appears the publish plugin works when using the convention plugin approach so we don't have to define this in the root - https://github.com/grails-plugins/grails-server-timing/tree/publish-changes
What I don't like:
- I think config.app-debug is too generic. It happens to only configure debug now, but in my other projects, I also configure profiler settings, common bootRun jvm args, and even a runJar task to run like production would in a container. I'd rather we keep this more generically named to encourage us to centralize all of the BootRun configuration in one spot.
|
As TODO, once we merge, we still need to update the documentation (which I'm happy to do). The only reason I added it to this project was to make it easier to find stuff. Maybe it belongs outside of this project (the project structure stuff) |
| assetDevelopmentRuntime 'org.webjars.npm:bootstrap' | ||
| assetDevelopmentRuntime 'org.webjars.npm:bootstrap-icons' | ||
| assetDevelopmentRuntime 'org.webjars.npm:jquery' | ||
| add('assetDevelopmentRuntime', 'org.webjars.npm:bootstrap') |
There was a problem hiding this comment.
Was this because intellij didn't detect the scope? With the asset plugin defined in the same file, this seems unnecessary
There was a problem hiding this comment.
Yes, that was the reason. I can revert it if you like it better the DSL way.
There was a problem hiding this comment.
I'm just worried about consistency. It's a shame that IntelliJ seems to always half implements this stuff. What are other people's thoughts? @jamesfredley @sbglasius
There was a problem hiding this comment.
I'm going to go ahead and merge this, if @jamesfredley or @sbglasius has any feedback on this we can address later.
0820e08 to
5154fe7
Compare
No description provided.