feature: add from first reference to ResourceID too #846
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
| @@ -13,6 +13,16 @@ public static ResourceID fromResource(HasMetadata resource) { | |||
| resource.getMetadata().getNamespace()); | |||
| } | |||
|
|
|||
| public static Optional<ResourceID> fromFirstOwnerReference(HasMetadata resource) { | |||
There was a problem hiding this comment.
I would still put this method on the Mappers class because it's not easily findable here, imo.
There was a problem hiding this comment.
Based on the current situation it would fit also there. But ResourceID already has fromResource static initializer. A lean usually to have those methods directly in the class, basically an utility class is always harder to find (actually already a problem if there is need to search for it).
Currently Mapper contains static init methods for PrimaryResourceRetriever, these might be also directly on that interface, so its obviouse how to create them and not search for Mappers.
So I kinda lean towards this, to have it as it is now. If you insist we can move it however, ok with that too.
There was a problem hiding this comment.
Ah yes, and the method doesn't have the same exact semantics so yes, it makes kind of sense to keep it on ResourceID. Diff reviews sometimes make you miss context… 🤦🏼
see: https://github.com/strimzi/kafka-access-operator/pull/2/files#r786855066
It is useful situations like this.