buildpacks / libcnb

A non-opinionated language binding for the Cloud Native Buildpack Buildpack and Extension specifications
Apache License 2.0
32 stars 13 forks source link

Make buildpack API generic #296

Closed dmikusa closed 2 days ago

dmikusa commented 2 weeks ago

Instead of using interface{} for things like metadata, we can use generic types. The buildpack API doesn't care what's in those values, so it can just be an any type. This will allow buildpack authors to set the types they want to use and even use custom types for their metadata. Then when accessing the metadata in their buildpacks, no casting is required. This reduces the amount of code and makes the code less error prone.

dmikusa commented 2 weeks ago

@samj1912 What do you think about this? I did this partially to play around with generics in Go, so if you don't think it improves things, let me know and that's totally fine.

I just know we have some casting in our Paketo buildpacks because of the metadata, so it seems like it'd be a nice win if we didn't have to do all that casting and generics makes that possible.

It does add some complexity to the libcnb code/API, but I think as a buildpack author it's not too bad. You won't see that complexity. I haven't done this yet, but if we go ahead with this idea, I will plan to add some helper constructors to make migrating easier. Basically, the method can assume the generic type is interface{} and if authors want to stick with that then they don't need to worry about generic types.

Anyway, I would appreciate your thoughts. Especially if you've used Go generics elsewhere and have thoughts about them and problems they might introduce. Thanks

dmikusa commented 2 days ago

I've been thinking about this more and I don't think it creates a lot of value. I'm going to withdrawal this PR. If someone feels strongly that we should go in this direction, please feel free to drop us a comment.