Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separating the gather logic into separate files #257

Merged
merged 5 commits into from Nov 16, 2020

Conversation

0sewa0
Copy link
Contributor

@0sewa0 0sewa0 commented Nov 10, 2020

BE AWARE THIS IS A FAT ONE.

So to save time and pain in the future there is a major refactor necessary for the clusterconfig.go. Its an unmaintainable mess of code thats 2000 lines long. This PR splits it up into individual files.

There are still some problematic parts like the 0-utils.go which contains all the code that was shared between the gather functions and it should be refactored further. versions.go is also problematic because it has 2 gather functions in it, one of which relies on the other, which we don't want.

Also gather-clients are now created where they will be used and not passed down from three levels higher. Only the configs and context is provided to the gather-functions.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 10, 2020
@0sewa0
Copy link
Contributor Author

0sewa0 commented Nov 11, 2020

/retest

1 similar comment
@0sewa0
Copy link
Contributor Author

0sewa0 commented Nov 11, 2020

/retest

@0sewa0
Copy link
Contributor Author

0sewa0 commented Nov 11, 2020

/test ci/prow/insights-operator-e2e-tests

@openshift-ci-robot
Copy link
Contributor

@0sewa0: The specified target(s) for /test were not found.
The following commands are available to trigger jobs:

  • /test e2e
  • /test e2e-gcp-upgrade
  • /test images
  • /test insights-operator-e2e-tests
  • /test unit

Use /test all to run all jobs.

In response to this:

/test ci/prow/insights-operator-e2e-tests

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.

@0sewa0
Copy link
Contributor Author

0sewa0 commented Nov 11, 2020

/test insights-operator-e2e-tests

1 similar comment
@0sewa0
Copy link
Contributor Author

0sewa0 commented Nov 12, 2020

/test insights-operator-e2e-tests

@rluders
Copy link
Contributor

rluders commented Nov 16, 2020

Tested locally. And... it works. 👍
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 0sewa0, rluders

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

5 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 5ddfdac into openshift:master Nov 16, 2020
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.

None yet

6 participants