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

cmd/ansible-operator,cmd/helm-operator: embed code in main.go #3415

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

joelanford
Copy link
Member

Description of the change:

  • Remove ansible.Run() and helm.Run() functions and move code to refactored ansible and helm main.go files
  • Remove shared WatchFlags and duplicate those flags in both the ansible and helm operator flag structs.

Motivation for the change:

  • Ansible and Helm operator code should be more self-contained in preparation for 1.0 release.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
@jmrodri
Copy link
Member

jmrodri commented Jul 15, 2020

At first I didn't like this idea, I prefer a small main.go with another layer to do most of the work, but this looks okay to me. This might impact @VenkatRamaraju PR #3374

Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@joelanford joelanford merged commit 0cc777e into operator-framework:master Jul 15, 2020
@joelanford joelanford deleted the ansible-helm-main branch July 15, 2020 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

4 participants