go-semantic-release / semantic-release

📦🚀 semantic-release written in Go
https://go-semantic-release.xyz
MIT License
395 stars 43 forks source link

support nested hook plugin configuration #163

Closed superewald closed 10 months ago

superewald commented 10 months ago

The current implementation of the config reader doesn't support nested configurations for hook plugins. Also, two plugins using the same option key would cause a problem and render one of the plugins unusable.

proposal

// change the HooksPlugins to PluginHookConfig array type Config struct { HooksPlugins []PluginHookConfig }


- use mapstructure to parse the options in the plugin
```go
type ExecPluginOptions struct {
    OnSuccess   string
    OnNoRelease string
}

func (pl *Plugin) Init(opts interface{}) {
    // parse options using mapstructure
    var execOpts ExecPluginOptions
    if err := mapstructure.Decode(opts, &execOpts); err != nil {
        // handle error
    }
}

considerations

  1. maybe decoding the options to the structure can be done in semantic-release itself before calling Hook.Init by reflecting the argument type of the Init function; I'm not sure if that works with the go-plugins library

  2. for backwards compatibility add a check if plugins.hooks.names exist; if so populate HooksPlugins by hand (otherwise it could be unmarshalled using viper)

    • the options itself are backward compatible because mapstructure allows to decode to map[string]string
  3. this would also work for other plugins and allow the config to be unmarshalled completely instead of parsing it by hand. In my opingion the config struct could look much cleaner this way; thoughts on this ?

christophwitzko commented 10 months ago

Hey @superewald, thank you for this elaborate feature request and proof of concept. ☺️

The idea behind using a shared config between all hooks plugins was to keep the config file as simple as possible. Additionally, the very generic map[string]string type allows the plugins to take care of stronger type validation (e.g. as you suggest, they could use a library like mapstruct).

When two plugins use the same option key, this could mean either:

  1. They are doing a similar job and probably should not be used together.
  2. The naming of the options key is not explicit enough (e.g., generic names like enable or file) and thus causes conflicts.
  3. The option key can actually be shared between the plugins, like cool_provider_account_id.

I think the issue could be solved by encouraging plugin authors to choose better option names and document potentially conflicting plugins.

To summarize, I prefer the config format as it is for now. Nevertheless, I really would like to hear an example where you discovered conflicting hooks options.

Best, Chris