operator-framework / deppy

Deppy: The dependency resolver for Kubernetes
Apache License 2.0
14 stars 21 forks source link

Breaking change: Remove getter methods on constraints and expose fields instead #154

Closed m1kola closed 5 months ago

m1kola commented 6 months ago

Today constraints hold important data in unexported fields and provide getters to access data.

It seems like there is not much value in these getters: Deppy itself doesn't seem to use them and on the user side they are normally not required in production code.

However this makes testing of components which generate variables with constraints harder for deppy users. Users have to use something like go-cmp with AllowUnexported & use constructors such as constraint.AtMost() to create constraints instead of simply just comparing literals.

I think we should explore exporting constraint fields and removing getter methods.

For example, AtMostConstraint which looks today like this:

https://github.com/operator-framework/deppy/blob/3a99ed74aa56e34e0a4beba81fe12e4b7dae4e06/pkg/deppy/constraint/constraint.go#L159-L177

will be turned into something like this:

type AtMostConstraint struct { 
    IDs []deppy.Identifier 
    N   int 
 } 

 func (constraint *AtMostConstraint) String(subject deppy.Identifier) string { 
    s := make([]string, len(constraint.IDs)) 
    for i, each := range constraint.IDs { 
        s[i] = string(each) 
    } 
    return fmt.Sprintf("%s permits at most %d of %s", subject, constraint.N, strings.Join(s, ", ")) 
 }