Added enum values to ids for ChangedEnumTrait events #1807
Conversation
| @@ -84,7 +84,7 @@ private List<ValidationEvent> validateEnum(ChangedShape<Shape> change, Pair<Enum | |||
| .severity(Severity.ERROR) | |||
| .message(String.format("Enum value `%s` was removed", definition.getValue())) | |||
| .shape(change.getNewShape()) | |||
| .id(getEventId() + REMOVED) | |||
| .id(getEventId() + REMOVED + definition.getValue()) | |||
There was a problem hiding this comment.
Enum values support arbitrary content, so this seems problematic to include them in the ID. Ideally these events would use the name of an enum definition if available, and if not... I dunno. Maybe the index of the entry? Or a name derived from the value?
There was a problem hiding this comment.
I'm okay with always using the index if you think that's acceptable
There was a problem hiding this comment.
Yeah sounds good. The caveat I can think of is that what is, say index 2, today won't be index 2 in the future if index 2 is removed. Not sure if that matters for you.
There was a problem hiding this comment.
Shouldn't be an issue since we only care about these events in the scope of a single Trebuchet feature and not after that
Issue #, if available:
Description of changes:
Adds the enum indices to the event ids
eg.
ChangedEnumTrait.Removed.4By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.