ui: Remove class-map reference from 1.11 #12358
Conversation
| (array 'empty' (eq checks.length 0)) | ||
| (array status (not-eq checks.length 0)) | ||
| }} | ||
| class={{concat 'consul-instance-checks' (if (eq checks.length 0) ' empty') (if (not-eq checks.length 0) (concat ' ' status))}} |
There was a problem hiding this comment.
Looks like this could maybe be simplified to one if/else condition like (if (eq checks.length 0) ' empty' (concat ' ' status)) but def not a blocker
There was a problem hiding this comment.
Hey, so I decided to leave this as is, it's pretty much deadcode now anyway (1.12 is different to this an wioll always be different moving forward). Plus if there are any changes upstream than need to come down here also (unlikely but you never know), the code here is logically as close as possible to the 1.12 version without using the helper that only exists in 1.12. For example if we needed to add another class in 1.12 we wouldn't be able to use an if/else here, we'd have to revert it to as it is now.
All in all I getcha completely on the query, and also that it's not a blocker, thought I'd give my reasoning for leaving it as is. Thanks again for the review!
johncowen
added
the
pr/no-changelog
|
Added a no-changelog here as our changelog checker doesn't work if the base branch isn't |
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/589395. |
Fixes #12351
A new helper added for 1.12 work unfortunately ended up in our 1.11 branch. This removes the helper and replaces it with equivalent code.