Approval: Clarify possible values #45
Conversation
|
/assign @dinhxuanvu |
|
@bipuladh @exdx @dinhxuanvu can one of you please review this? Its a tiny change that helps a lot with usability. Also, it would be awesome if this repo would get an OWNERS file so it becomes clear who to assign changes to. |
crds/zz_defs.go
Outdated
| } | ||
|
|
||
| info := bindataFileInfo{name: "operators.coreos.com_catalogsources.yaml", size: 7231, mode: os.FileMode(420), modTime: time.Unix(1591650274, 0)} | ||
| info := bindataFileInfo{name: "operators.coreos.com_catalogsources.yaml", size: 7231, mode: os.FileMode(436), modTime: time.Unix(1593116031, 0)} |
There was a problem hiding this comment.
this changed as a result of running codegen? A little confused as to why file permissions would change as a result of adding a comment to the source code.
There was a problem hiding this comment.
yeah, as a result of make generate. I don't really understand this, I would not have expected this change but instead a change of the generated openapi schemes in crds/operators.coreos.com_installplans.yaml and crds/operators.coreos.com_subscriptions.yaml. To me, it looks like either I am using this somehow wrong or its broken. I was hoping there would be a job that fails and tells me what to do but doesn't seem to be the case 🙃
There was a problem hiding this comment.
This is just a bit strange. Maybe you can discard these changes from the PR and just keep the main change in pkg/operators/installplan_types.go. Then I can lgtm and merge this PR. Thanks.
There was a problem hiding this comment.
@dinhxuanvu updated, PTAL
|
@exdx can you advise on who to ping to get this merged? |
crds/zz_defs.go
Outdated
| } | ||
|
|
||
| info := bindataFileInfo{name: "operators.coreos.com_catalogsources.yaml", size: 7231, mode: os.FileMode(420), modTime: time.Unix(1591650274, 0)} | ||
| info := bindataFileInfo{name: "operators.coreos.com_catalogsources.yaml", size: 7231, mode: os.FileMode(436), modTime: time.Unix(1593116031, 0)} |
There was a problem hiding this comment.
This is just a bit strange. Maybe you can discard these changes from the PR and just keep the main change in pkg/operators/installplan_types.go. Then I can lgtm and merge this PR. Thanks.
|
@dinhxuanvu updated, ptal |
|
/lgtm |
Currently this field does not seem to get defaulted (should it?) and the docs are pretty useless because the name is self-explanatory for the what, but the how is not documented: