kube-object-storage / lib-bucket-provisioner

Library for the dynamic provisioning of object store buckets to be used by object store providers.
Apache License 2.0
21 stars 22 forks source link

`error` return value is always nil #135

Closed copejon closed 4 years ago

copejon commented 5 years ago

No process within NewProvisioner() generates and error; the function will only ever return nil.

Let's remove the error return value from the function so consumers don't end up writing unnecessary error checks.

guymguym commented 5 years ago

It's really a non-issue, but still I don't think it's the right thing to do. Making such changes for saving the error checks that are probably just occurring in a single location in the code of every provider is not worth removing the error from the main initialization API call of a library. This just limits the flexibility of the library to improve. If providers don't want to handle errors explicitly in initialization they can panic or fatal. CC @travisn

travisn commented 5 years ago

Agreed that it's a very minor issue, but philosophically speaking...

If an API has no need to return an error, why return a placeholder that will always be nil? It will require unnecessary error handling by the caller.

With golang an API can change over time. Clients that need to update to the latest API would update their vendoring and then change their calls accordingly. So there is nothing preventing the API from changing if an error condition is added in the future.

guymguym commented 5 years ago

I philosophically don't mind it either way :)

BTW this is a SEMVER MAJOR change.

guymguym commented 5 years ago

I'm removing the "bug" label

copejon commented 4 years ago

Those are all pretty compelling reasons not to mess with it right now, closing.