operator-framework / deppy

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

Replace commas in error messages #150

Closed m1kola closed 6 months ago

m1kola commented 6 months ago

We need to make errrors more readable for human consumption.

Currently with commans it is difficult to read the error when there is DependencyConstraint or AtMostConstraint constraint in the problem as they also use a comma to separate ids:

constraints not satisfiable: installed package prometheus is mandatory, installed package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0, prometheus package uniqueness permits at most 1 of test-catalog-prometheus-prometheus-operator.2.0.0, test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0, required package prometheus is mandatory, required package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.2.0.0

AtMostConstraint is in bold.

We should explore different formatting options (such as semicolon or new lines).

Semicolon will already be a big improvement:

constraints not satisfiable: installed package prometheus is mandatory; installed package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0; prometheus package uniqueness permits at most 1 of test-catalog-prometheus-prometheus-operator.2.0.0, test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0; required package prometheus is mandatory; required package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.2.0.0

joelanford commented 6 months ago

I know I originally suggested semi-colons, but what do you think about new lines instead of semi-colons? How does that end up looking in kubectl get operators -o yaml or kubectl describe operators.

Semi-colons are definitely an improvement over commas, but if newline based output is more readable using the above commands, maybe we jump straight to that?

If not, or if there is more than trivial discussion, +1 on semi-colons over commas.

m1kola commented 6 months ago

Happy for us to explore new lines. I'm pretty sure that kubectl will be ok with new lines, but it is worth double checking. I updated the description of the issue to be less prescriptive.

I also created #151 for the bigger effort of making errors more concise and useful. Let's keep the scope of this issue to tweaks for separators.