Format .proto files with spotlessApply #13
Conversation
- Use the buf-gradle-plugin to download the correct buf binary automatically and configure spotless to use it - Update spotless plugin to `7.0.0` - Use spotless to format files in this repo Signed-off-by: Akshath Kothari <[email protected]>
build.gradle.kts
Outdated
| @@ -6,6 +6,7 @@ plugins { | |||
| id("org.hypertrace.ci-utils-plugin") version "0.3.0" | |||
| id("org.hypertrace.publish-plugin") version "1.0.4" | |||
| id("org.owasp.dependencycheck") version "8.4.0" | |||
| id("com.diffplug.spotless") version "7.0.0" | |||
There was a problem hiding this comment.
Rather than using spotless directly you could just use the last version of this plugin to have been published to dogfood it a bit.
There was a problem hiding this comment.
I tried to use the current version and settled on this, but that's a good idea to use the last version.
| project | ||
| .getConfigurations() | ||
| .getByName(BufSupportKt.BUF_BINARY_CONFIGURATION_NAME) | ||
| .getSingleFile(); |
There was a problem hiding this comment.
We don't typically want to do something like this since it forces the buf binary to be resolved whether the user is formatting or not. If the buf plugin is responsible for downloading it, are they not already making it executable? Can you use a provider instead?
There was a problem hiding this comment.
They're not making it executable. They do the same in their exec task: https://github.com/bufbuild/buf-gradle-plugin/blob/f03bbc1e5cf902eaf9c505d0b43d603aee6f5a85/src/main/kotlin/build/buf/gradle/BufSupport.kt#L74-L76
I can switch to a provider.
There was a problem hiding this comment.
Updated to use a provider, let me know if that was what you were expecting.
Signed-off-by: Akshath Kothari <[email protected]>
build.gradle.kts
Outdated
| @@ -4,6 +4,7 @@ plugins { | |||
| `java-gradle-plugin` | |||
| id("org.hypertrace.repository-plugin") version "0.4.0" | |||
| id("org.hypertrace.ci-utils-plugin") version "0.3.0" | |||
| id("org.hypertrace.code-style-plugin") version "latest.release" | |||
There was a problem hiding this comment.
We always want versions pinned. By latest version, I meant the actual version that was most recent :) Otherwise a breaking change could break the build.
| format -> | ||
| format | ||
| .buf(bufExtension.getToolVersion()) | ||
| .pathToExe(bufBinaryProvider.get().getAbsolutePath())); |
There was a problem hiding this comment.
so the provider part is right, but if pathToExe doesn't accept a provider we don't get the benefit we're looking for - we want gradle to be resolving the provider JIT so .get() is usually a red flag.
This actually is a closure already (the lambda from 67) it's just one that likely evaluates during config time rather than execution. So moving the resolution inside here appears to be the best we can do. Given that, we have the option of either removing the explicit provider and moving that code inside this lambda or leaving it as is. They should be equivalent, so up to you which you feel is more readable.
There was a problem hiding this comment.
Got it, I see what you mean now. I believe the spotless gradle plugin supports lazy configuration, so it shouldn't try to configure the protobuf plugin if the user is not formatting files. I'll revert my last commit that moved the resolution from this lambda to a provider.
|
Spotted a few issues with this - bufLint runs automatically and fails for subprojects which don't have any proto files. The buf.yaml file is also ignored which should probably be fine but we should probably delete it if it's not used. Converted to draft for now, will revisit this in a while. |
7.0.0(released today)