Look at PRR approval request when querying KEPs #2397
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnbelamaric The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
pkg/kepctl/kepctl.go
Outdated
| prrFile, err := os.Open(prrPath) | ||
| if !os.IsNotExist(err) { | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not open file %s: %v\n", prrPath, err) |
There was a problem hiding this comment.
the flow used to return without error when the prr yaml wasn't present. Seems like we still want to allow that case, right?
There was a problem hiding this comment.
it still does. if !os.IsNotExists line above
There was a problem hiding this comment.
different style.
I'm used to
if os.IsNotExist(err){
return existing
}
if err != nil{
fail
}
code parsing new values. There was a problem hiding this comment.
yeah, that's better.
There was a problem hiding this comment.
that's what I would normally do, don't know why i did it this way
|
/lgtm Just a question more than an issue. This worked locally for me (I think). /hold |
24100e4 to
cf7d2fb
Compare
|
/lgtm |
|
/hold cancel |
|
lgtm - thanks |
While querying for KEPs needing PRR approval, I saw we were still missing some KEPs. This is because people aren't setting
prrApproversinkep.yamland are instead just setting it in the PRR approval request yaml. This hacky PR allows us to search those too./assign @wojtek-t @deads2k
/cc @jeremyrickard