Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Created app should use groovy gradle styling #398

Open
codeconsole opened this issue Oct 11, 2024 · 9 comments · Fixed by #424
Open

Created app should use groovy gradle styling #398

codeconsole opened this issue Oct 11, 2024 · 9 comments · Fixed by #424
Assignees

Comments

@codeconsole
Copy link
Contributor

codeconsole commented Oct 11, 2024

Current app looks like Kotlin and adds a bunch of unnecessary characters.

Not sure why this happened, but if we are going to keep this route we should also add semi-colons to every line.

dependencies {
    implementation("org.grails:grails-core")
    implementation("org.grails:grails-logging")
    implementation("org.grails:grails-plugin-databinding")
    implementation("org.grails:grails-plugin-i18n")
    implementation("org.grails:grails-plugin-interceptors")
    implementation("org.grails:grails-plugin-rest")
    implementation("org.grails:grails-plugin-services")
    implementation("org.grails:grails-plugin-url-mappings")
    implementation("org.grails:grails-web-boot")

Formatting should be the same as it always has been:

dependencies {
    developmentOnly "org.springframework.boot:spring-boot-devtools"
    compileOnly "io.micronaut:micronaut-inject-groovy"
    console "org.grails:grails-console"
    implementation "org.springframework.boot:spring-boot-starter-logging"
    implementation "org.springframework.boot:spring-boot-starter-validation"
    implementation "org.springframework.boot:spring-boot-autoconfigure"
    implementation "org.grails:grails-core"
    implementation "org.springframework.boot:spring-boot-starter-actuator"
    implementation "org.springframework.boot:spring-boot-starter-tomcat"

centralize dependency versions in gradle.properties so they are not repeated multiple times.

@jeffscottbrown
Copy link
Member

or bonus points if you use Strings instead of GStrings

FYI... I think the examples you cited are indeed Strings. I don't think Groovy creates GStrings for something like "org.grails:grails-console". I think the compiler knows that is a String.

@codeconsole
Copy link
Contributor Author

codeconsole commented Oct 31, 2024

or bonus points if you use Strings instead of GStrings

FYI... I think the examples you cited are indeed Strings. I don't think Groovy creates GStrings for something like "org.grails:grails-console". I think the compiler knows that is a String.

Thanks for clarifying @jeffscottbrown
I tested and confirmed the compiler is smart enough to compile into a String so I now prefer double quotes, but obviously without the braces.

implementation "org.springframework.boot:spring-boot-starter-logging"

@matrei
Copy link
Contributor

matrei commented Oct 31, 2024

It's obviously a personal preference, but for what it's worth, I prefer single quotes as it helps my brain distinguish between String and GString without having to check. Also, again my opinion, single quotes take up less space, which makes the code easier to read.

@codeconsole
Copy link
Contributor Author

@matrei I agree, but knowing now it always compiles to a String changed my perspective because

  1. The build file no longer has uniform formatting
  2. If you want to add a dynamic version, you have to edit both quotes as opposed to just add the version.
assert "Test" instanceof String 
assert "Test" instanceof GString  // <-- Fails

@matrei
Copy link
Contributor

matrei commented Nov 1, 2024

The build file no longer has uniform formatting

That's part of the feature, I don't have to actively check if there is a variable in the string when reading/scanning the code.

If you want to add a dynamic version, you have to edit both quotes as opposed to just add the version.

This is the drawback, but with Alt+Enter+Enter in Intellij you can toggle between GStrings and String syntax easily.
Are we optimizing for reading or writing? I think every little thing that brings clarity and simplifies reading helps out.

@jamesfredley
Copy link
Contributor

You need double quotes for versions from gradle.properties.

If we want them to match, double quotes would be the way to go. With the grails-bom expansion, these are rarer, but there will be a few in a normal Grails apps.

Some examples from one of my projects:

implementation "io.awspring.cloud:spring-cloud-starter-aws:$springCloudStarterAWSVersion"
implementation "io.awspring.cloud:spring-cloud-starter-aws-parameter-store-config:$springCloudStarterAWSVersion"
implementation "com.sendgrid:sendgrid-java:$sendGridVersion"

implementation "com.google.api-client:google-api-client:$googleApiClientVersion"
implementation "com.google.auth:google-auth-library-oauth2-http:$googleOauth2Version"
implementation "com.google.maps:google-maps-services:$googleMapsServicesVersion"

@codeconsole
Copy link
Contributor Author

codeconsole commented Nov 9, 2024

I've spent a lot of time thinking about this and I now advocate for double quotes.

My rational is for consistency across the entire file and still accomodate situations that need GStrings. It is also from the perspective of clarity for people that are not fluid in the Groovy language.

Previously I adopted the Groovy purist perspective where '' was always used for Strings, but considering the fact Groovy has lost a lot of popularity and has tried in several ways to be more like Java over the past 5 years, I think it is better to preference Java like constructs when the Groovy equivalent does not provide any gains. This is more to eliminate any confusion to someone new to the Groovy language.

@matrei
Copy link
Contributor

matrei commented Nov 9, 2024

Sounds like the majority favors double quotes ☺️

@codeconsole
Copy link
Contributor Author

lol, I still like single quotes, but if Groovy isn't going to have a purist mentality when it compiles double quotes into Strings, why should I?

@jamesfredley jamesfredley linked a pull request Nov 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

4 participants