Skip to content

Conversation

@cgwalters
Copy link
Member

We haven't changed how the renderer works in forever, but a lot
of people keep changing the templates. This causes pointless churn
in the test data and makes changing the templates unnecessarily
painful.

Further, I want to change how the rendering works to inherit
from a common base, and having the unit test code also parsing
the templates is problematic.

Let's just sanity check that we have the pull secret and kubelet
unit.

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 14, 2019
We haven't changed how the renderer works in forever, but a lot
of people keep changing the templates.  This causes pointless churn
in the test data and makes changing the templates unnecessarily
painful.

Further, I want to change how the rendering works to inherit
from a common base, and having the unit test code also parsing
the templates is problematic.

Let's just sanity check that we have the pull secret and kubelet
unit.
@cgwalters cgwalters force-pushed the template-test-sanity branch from 222bdda to c3245b7 Compare May 14, 2019 17:59
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2019
@runcom
Copy link
Member

runcom commented May 14, 2019

love this

@kikisdeliveryservice
Copy link
Contributor

/test e2e-aws

@kikisdeliveryservice
Copy link
Contributor

This makes a lot of sense to me!

foundPullSecretMaster := false
foundPullSecretWorker := false
foundKubeletUnitMaster := false
foundKubeletUnitWorker := false
Copy link
Member

@runcom runcom May 14, 2019

Choose a reason for hiding this comment

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

this is the most stupid nits every but declaring a boolean in Golang is the same as setting it to false so var found......, found .... , ..... bool is the same here (and this nit can live here for the time being, just wanted to point it out :D

@runcom
Copy link
Member

runcom commented May 14, 2019

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, runcom

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

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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants