🌱 (chore) add comment to clarify usage context of ReplaceRegexInFile #4656
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
size/S
|
Hi @kersten. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
camilamacedo86
changed the title
There was a problem hiding this comment.
If we move forward here it will be a breaking change, see: https://github.com/theishshah/ocp-release-operator-sdk/blob/9a49219477ed0f6ddc1c9e5e885cbfedffb51b64/internal/testutils/scorecard.go#L90
So, we need to keep this method since it is used by other projects.
Therefore, are you OK with we close this one ?
|
Sure, if it's needed, then it should be kept. But let me revert the change and put a comment in the code. |
k8s-ci-robot
added
size/XS
kersten
changed the title
pkg/plugin/util/util.go
Outdated
| @@ -245,6 +245,8 @@ func ReplaceInFile(path, oldValue, newValue string) error { | |||
|
|
|||
| // ReplaceRegexInFile finds all strings that match `match` and replaces them | |||
| // with `replace` in the file at path. | |||
| // | |||
| // Deprecated: remove it when the usage of this function is removed by other projects. | |||
There was a problem hiding this comment.
I don't know if we should either deprecate it.
It was added as a helper function we might want to use in our code implementation as leave it available for others.
There was a problem hiding this comment.
Ah, didn't know that. I pushed another commit, reflecting that? Would this be ok?
kersten
force-pushed
the
chore/remove-unused-replaceregexinfile
branch
from
00801e7 to
4beba92
Compare
kersten
changed the title
camilamacedo86
changed the title
| @@ -245,6 +245,9 @@ func ReplaceInFile(path, oldValue, newValue string) error { | |||
|
|
|||
| // ReplaceRegexInFile finds all strings that match `match` and replaces them | |||
| // with `replace` in the file at path. | |||
| // | |||
| // This function is currently unused in the Kubebuilder codebase, | |||
| // but is used by other projects and may be used in Kubebuilder in the future. | |||
There was a problem hiding this comment.
Can you please sqush the commits for we move forward with this one?
Update the comment to note that the function is currently unused in the Kubebuilder codebase, but used by other projects and may be used internally in the future.
kersten
force-pushed
the
chore/remove-unused-replaceregexinfile
branch
from
4beba92 to
20187d8
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, kersten 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 |
k8s-ci-robot
added
the
approved
Update the comment to note that the function is currently unused in the Kubebuilder codebase, but used by other projects and may be used internally in the future.