Move clean config option to the top level of generation config #3165
Conversation
|
The latest Buf updates on your PR.
|
| // Always expect non-fake data, because the existing ".jar" or ".zip" | ||
| // file is always replaced by the output. This is the existing and correct | ||
| // behavior. | ||
| require.True(t, len(data) > 1, "expected non-fake data at %q", fullOutputPath) |
There was a problem hiding this comment.
Flagging this. It seems the clean option doesn't affect .jar and .zip outputs since they have always been cleaned (the generator rewrites the .jar or .zip file), prior to the --clean flag and clean option.
| // Clean, if set to true, will delete the output directories, zip files, or jar files | ||
| // before generation is run. | ||
| Clean bool `json:"clean,omitempty" yaml:"clean,omitempty"` |
There was a problem hiding this comment.
Since this is at the top level, I think we might need to make the key a little more specific. I know it looks a little longer, but just clean might not be very clear without referring to the docs. Maybe we should name it clean_plugin_outs? ^^"
Co-authored-by: Doria Keung <doriable@users.noreply.github.com>
| @@ -22,6 +22,9 @@ import ( | |||
|
|
|||
| // GenerateConfig is a generation configuration. | |||
| type GenerateConfig interface { | |||
| // DeleteOuts is whether to delete the output directories, zip files, or jar files before | |||
| // generation is run. | |||
| DeleteOuts() bool | |||
There was a problem hiding this comment.
Would keep this consistent with the name clean_plugin_outs
| @@ -38,6 +41,7 @@ type GenerateConfig interface { | |||
|
|
|||
| // NewGenerateConfig returns a validated GenerateConfig. | |||
| func NewGenerateConfig( | |||
| clean bool, | |||
There was a problem hiding this comment.
Would name this deletePluginOuts to match the generateConfig structure.
| @@ -55,6 +60,7 @@ func NewGenerateConfig( | |||
| // *** PRIVATE *** | |||
|
|
|||
| type generateConfig struct { | |||
| deleteOuts bool | |||
There was a problem hiding this comment.
Nit: would name this deletePluginOuts
This PR targets branch
add-buf-gen-yaml-cleanand movescleantobuf.gen.yaml's top level, from the plugin level.