bufbuild / buf-gradle-plugin

Gradle plugin for the Buf CLI
Apache License 2.0
49 stars 13 forks source link

Support other extensions for published image #170

Closed sato9818 closed 9 months ago

sato9818 commented 10 months ago

This PR adds new option that allows users to specify an image filename published by bufBuild. Users can specify other formats than json.

CLAassistant commented 10 months ago

CLA assistant check
All committers have signed the CLA.

andrewparmet commented 10 months ago

Two initial thoughts:

First, I see a list of file names that Buf recognizes in the docs:

Do you know if the current implementation of buf check breaking works on every format? I think we should add an integration test to cover each case.

Second, I'm thinking about the plugin API. Previously the contract was:

This change introduces a new contract:

On the whole both contracts are limiting if users want to use that file for purposes that aren't built-in to the plugin since we offer no way to grab the built image file without building a file path yourself. Given that the plugin exposes the file via the constant today, the README should be updated to call out the bufBuild task and how to get the built image file.

Stepping back I think it's a good feature to be able to produce an image in any supported format. This leaves a few action items before we can move forward:

Lots to think about. Thanks for taking on this new feature!

sato9818 commented 10 months ago

@andrewparmet Thank you for the kind and detailed review!

Firstly, I'm going to implement a integration test to check if buf check breaking works correctly.

Secondly, regarding the new plugin API, I think it's fine just to be able to configure the image extension. I agree with your opinion that we should let users choose their desired format and compression pair. I think we should control users to choose the pair from this list.

Lastly, I don't think we should unify three options: publishSchema, imageArtifactDetails and bufBuildPublicationFileName. In my usecase, I require the capability to generate an image file with a specified extension without publishing it.

andrewparmet commented 10 months ago

For this PR I think it's ok to leave the configuration APIs as they are, and I'll tweak them for some coherence afterwards.

Yes, that list is more thorough than what I saw.

May I ask what your use case is? I'm wondering if this plugin can more directly support it.

sato9818 commented 10 months ago

My team is using Armeria as a grpc framework, which takes a descriptor file for its doc service. But in Armeria, the json format descriptor is not supported. So, we would like other format than json like .bin.

sato9818 commented 10 months ago

@andrewparmet

I added some tests to check if buf breaking works correctly with other extensions than json.

It seems buf build supports only these extensions as outputs:

Could you please review the code again?

sato9818 commented 9 months ago

Hi @andrewparmet. I was wondering if there might be some time to take a look at this soon.

If there's anything I can do to help move it along, just let me know. I'm all ears for any feedback you might have.

sato9818 commented 9 months ago

@andrewparmet Thank you for your code change suggestion!

I changed the code to separate the extension option into a format and compression method option.

andrewparmet commented 9 months ago

Released in 0.9.0