improve (HLS)ValidationIssue

jaybpierce commented 4 years ago

Description

comment I ran across in our current implementation: // mamba validation issues have a poor interface. Best I can do is search in the description field when trying to find a specific issue in a list.

Tasks

[ ] change public init(description: IssueDescription, severity: IssueSeverity) to store the IssueDescription instead of just the description string
[ ] change public init(description: String, severity: IssueSeverity) to store the description as a .undefined(String) within IssueDescription enum
[ ] change description attribute to an accessor of the string of the stored IssueDescription

Open Questions

Does this seem like the right approach IYO @dcoufal ?

dcoufal commented 4 years ago

I'm not sure.

We have two opposing concerns.

(1) We'd like all users to be able to create custom ValidationIssues for their custom HLS tags. So, having a string is useful for that. (2) We'd like to be able to id specific validation issues in code. An enum is best for that.

I wonder if we'd be better off with a IssueDescription protocol instead? Something like:

protocol IssueDescription {
   var description: String {get}

   func isEqual(to: IssueDescription) -> Bool
}

Users would probably implement this via a String enum. Then everyone could define their own, but by using the isEqual hack (which would have to do some casting, which is gross but useful) you'd also be able to tell what specific issue you are having.

Anyway, I'm also not eager to foist a second breaking change on our users so soon after 2.0 (this would have to be 3.0 if we just did it without a transition plan).

jaybpierce commented 4 years ago

"Anyway, I'm also not eager to foist a second breaking change on our users so soon after 2.0 (this would have to be 3.0 if we just did it without a transition plan)."

totally agree with that, just wanted to bring it up since I ran across the comment

Comcast / mamba