DEVPROD-6656 surface distro warnings in validator #7913
Conversation
validator/project_validator.go
Outdated
| distroWarnings[distroName] = warningNote | ||
| return | ||
| } | ||
| distroWarnings[distroName] = fmt.Sprintf("%s; %s", distroWarnings[distroName], warningNote) |
There was a problem hiding this comment.
Out of curiosity, does the UI expect these to be presented as a long semicolon-delimited list? I would have thought it would separate them by newlines or something.
There was a problem hiding this comment.
I feel like its unlikely that the case comes up where there are multiple warnings, but perhaps new line in between would be clearer. I can update to that. (I think if there are multiple warnings its likely to be ugly no matter what unfortunately)
| Message: fmt.Sprintf("task '%s' in buildvariant '%s' "+ | ||
| "references distro '%s' with the following warning(s): %s", | ||
| task.Name, buildVariant.Name, name, warning), | ||
| Level: Warning, |
There was a problem hiding this comment.
Just for my understanding, these are not actual warnings from the distro validator, correct? They're just warnings that distro admins can manually set to give notice to users about some kind of issue?
Should we give some kind of heads up that users might see distro warnings or maybe that evergreen validate could give emit distro-related warnings? I know some teams use evergreen validate to check the validity of their YAMLs, but if they get this kind of warning, there's not much they can do about it because the warning comes from the distro, not their YAML.
There was a problem hiding this comment.
Admins haven't actually started using this yet so we don't need to give a heads up yet anyway, but I think that's a good point. I can tweak this wording, and surface with the infra team that they may want to communicate if they're going to be using frequently (and to be careful about usage -- like they should probably only use this if the distro is being removed soon or something).
There was a problem hiding this comment.
They could address the warning by switching distros, which I think is the point.
DEVPROD-6656
Description
UI added a distro warning field but this needs to be added to the validator before it makes sense to surface there.
A follow up PR will unhide this field on the UI.
Testing
Added unit tests
Documentation
Will make sure the platform team is aware of this feature so that they can make use of it.