[Merged by Bors] - Added OpaConfig struct #357
Conversation
src/opa.rs
Outdated
| /// | ||
| /// # Arguments | ||
| /// * `resource` - The cluster resource. | ||
| pub fn package_url<T>(&self, resource: &T) -> String |
There was a problem hiding this comment.
Personally I'm not a big fan of this: having a (potentially) complex resource as a dependency (parameter) just to read one field from it is usually an anti pattern. Maybe we can introduce a ClusterName type or something like that.
There was a problem hiding this comment.
Its just something of type ResourceExt for the name() and namespace() methods. It defines the behavior as in the doc that we default to the resource name. That would not be true if we have a parameter cluster_name.
There was a problem hiding this comment.
yep, T is ResourceExt, I'd be fine with this
There was a problem hiding this comment.
Ah, I missed that. Well then ...
|
looks good to me; tests work, docs are nice etc. But I'll let razvan get back to this |
|
Also we need to wait for github actions to work again 😄 |
merged package_url and rule_url methods to document_url.
|
bors merge |
## Description closes #356 Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
Description
closes #356
Review Checklist
Once the review is done, comment
bors r+(orbors merge) to merge. Further information