Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve (HLS)ValidationIssue #78

Open
3 tasks
jaybpierce opened this issue Oct 29, 2019 · 2 comments
Open
3 tasks

improve (HLS)ValidationIssue #78

jaybpierce opened this issue Oct 29, 2019 · 2 comments

Comments

@jaybpierce
Copy link
Contributor

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
Copy link
Contributor

dcoufal commented Oct 29, 2019

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
Copy link
Contributor Author

"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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants