Skip to content

Conversation

@r4f4
Copy link
Contributor

@r4f4 r4f4 commented Feb 20, 2023

This type of reporting is disabled by default in golangci-lint. However, we used to enforce it when using golang/lint and I think it's useful to have.

  • Fix some recent cases that were only caught during backport.

@openshift-ci openshift-ci bot requested review from sadasu and zaneb February 20, 2023 09:18
@r4f4
Copy link
Contributor Author

r4f4 commented Feb 20, 2023

/hold
I need to fix all recent exported names without comments.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2023
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will conflict with my PR that deletes this code altogether, but if that's the price of stopping the bleeding I am ok with it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also rebase this if you prefer that your PR gets merged first.

@r4f4 r4f4 force-pushed the golint-enable-godoc-exported branch 3 times, most recently from 3fd54cb to afcf24e Compare February 20, 2023 10:13
@r4f4
Copy link
Contributor Author

r4f4 commented Feb 20, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2023
Copy link
Contributor

@barbacbd barbacbd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Have we decided what to do with Zane's comments?

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 20, 2023
@zaneb
Copy link
Member

zaneb commented Feb 20, 2023

Go ahead, I can rebase
(also please review my PR 😜)
/lgtm

@sadasu
Copy link
Contributor

sadasu commented Feb 22, 2023

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sadasu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 22, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD bb7e59b and 2 for PR HEAD afcf24e8664f4aa6c5514dcf197a484399ac7d9f in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD ed246d2 and 1 for PR HEAD afcf24e8664f4aa6c5514dcf197a484399ac7d9f in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 0a0721d and 0 for PR HEAD afcf24e8664f4aa6c5514dcf197a484399ac7d9f in total

@openshift-ci-robot
Copy link
Contributor

/hold

Revision afcf24e8664f4aa6c5514dcf197a484399ac7d9f was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2023
r4f4 added 2 commits February 23, 2023 09:47
This type of reporting is disabled by default in golangci-lint. However,
we used to enforce it when using golang/lint and I think it's useful to
have.
@r4f4 r4f4 force-pushed the golint-enable-godoc-exported branch from afcf24e to bac8413 Compare February 23, 2023 08:54
@r4f4
Copy link
Contributor Author

r4f4 commented Feb 23, 2023

Rebased to fix newly introduced

pkg/types/vsphere/conversion/installconfig.go:15:1: exported: exported function ConvertInstallConfig should have comment or be unexported (revive)
func ConvertInstallConfig(config *types.InstallConfig) error {
^ 

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2023
Copy link
Contributor

@barbacbd barbacbd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2023
@r4f4
Copy link
Contributor Author

r4f4 commented Feb 23, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD ddfa74f and 2 for PR HEAD bac8413 in total

@r4f4
Copy link
Contributor Author

r4f4 commented Feb 23, 2023

/override ci/prow/e2e-vsphere-ovn
Install succeeded, just a few e2e failures.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 23, 2023

@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-vsphere-ovn

Details

In response to this:

/override ci/prow/e2e-vsphere-ovn
Install succeeded, just a few e2e failures.

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/test-infra repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 23, 2023

@r4f4: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-shared-vpc bac8413 link false /test e2e-aws-ovn-shared-vpc
ci/prow/e2e-nutanix-sdn bac8413 link false /test e2e-nutanix-sdn
ci/prow/okd-scos-e2e-aws-upgrade bac8413 link false /test okd-scos-e2e-aws-upgrade
ci/prow/okd-e2e-aws-ovn-upgrade bac8413 link false /test okd-e2e-aws-ovn-upgrade
ci/prow/e2e-vsphere-upi-zones bac8413 link false /test e2e-vsphere-upi-zones
ci/prow/okd-e2e-aws-ovn bac8413 link false /test okd-e2e-aws-ovn

Full PR test history. Your PR dashboard.

Details

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/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 8575dc5 into openshift:master Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants