Include criteria which imply the required criteria in suggest output #614
Conversation
For most consumers using exclusively the default criteria, this will only impact the output for `safe-to-run` recommendations to mention that `safe-to-deploy` would also be applicable. For consumers using more complex criteria, this may increase the size of the suggest output more substantially. This change does impact JSON output, but only for strings generally intended for human consumption. If a script was previously parsing these strings it may be broken by the change. Fixes mozilla#613
src/criteria.rs
Outdated
| @@ -177,6 +177,16 @@ impl CriteriaMapper { | |||
| pub fn criteria_index(&self, criteria_name: CriteriaStr<'_>) -> usize { | |||
| self.index[criteria_name] | |||
| } | |||
|
|
|||
| /// Yields the indicies for all criteria which imply the the given criteria. | |||
There was a problem hiding this comment.
optional nitty nit: s/the the/the/
|
Thanks! LGTM (although I am not sure if it counts because I am not a real owner / reviewer of this project). |
There was a problem hiding this comment.
This looks good.
I'd like to bring up a complaint in the human-readable output that I've had prior to this PR. We say "recommended audits for safe-to-run (or safe-to-deploy):", however to me that sentence structure makes it sound like those are audits you need to do to meet some "safe-to-run" objective overall. This doesn't make sense in context since you should be told the criteria which the audits must meet, however it still throws me off when I read it. I think it's clearer if the wording is "recommended safe-to-run (or safe-to-deploy) audits:" or "recommended audits {meeting,satisfying} safe-to-run (or safe-to-deploy):".
Co-authored-by: Alex Franchuk <[email protected]>
For most consumers using exclusively the default criteria, this will only impact the output for
safe-to-runrecommendations to mention thatsafe-to-deploywould also be applicable. For consumers using more complex criteria, this may increase the size of the suggest output more substantially.This change does impact JSON output, but only for strings generally intended for human consumption. If a script was previously parsing these strings it may be broken by the change.
Fixes #613