Skip to content
This repository has been archived by the owner on Jun 24, 2020. It is now read-only.

Letting flags be changed from outside #223

Merged

Conversation

cardil
Copy link
Contributor

@cardil cardil commented Nov 14, 2019

Proposed Changes

  • Letting flags be changed from outside
    Changing those flags is needed to perform tests on RH operators.
  • Adding Intellij, and vandors excluded files to gitignore

Implementation

This PR let change test flags from outside. It can be done either by:

  • declaring environmental variables: TEST_NAMESPACE, and TEST_RESOURCE,
  • using -ldflags "-X knative.dev/serving-operator/test.ServingOperatorNamespace=knative-serving"

Release Note

NONE

@googlebot googlebot added the cla: yes Author(s) signed a CLA. label Nov 14, 2019
Copy link
Contributor

@knative-prow-robot knative-prow-robot left a comment

Choose a reason for hiding this comment

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

@cardil: 0 warnings.

In response to this:

Proposed Changes

  • Letting flags be changed from outside
    Changing those flags is needed to perform tests on RH operators.
  • Adding Intellij, and vandors excluded files to gitignore

Implementation

This PR let change test flags from outside. It can be done either by:

  • declaring environmental variables: TEST_NAMESPACE, and TEST_RESOURCE,
  • using -ldflags "-X knative.dev/serving-operator/test.ServingOperatorNamespace=knative-serving"

Release Note

NONE

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.

@knative-prow-robot
Copy link
Contributor

Hi @cardil. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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

@cardil
Copy link
Contributor Author

cardil commented Nov 14, 2019

This PR mirrors knative/eventing-operator#21

@@ -1,6 +1,13 @@
# Temporary Build Files
build/_output
build/_test
# Goland
.idea
Copy link
Member

Choose a reason for hiding this comment

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

not 100% sure on getting this in.... I guess... shouldn't those be on your own global .gitignore ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean by using git config --global core.excludesfile option?

I think not many people actually use that. I didn't hear of that option, nor any of developers in my office (I know that's not representative 😉)

On top that it is easier for newcomers, so that repo declares those ignores.

vendor/**/OWNERS
# Test Infra Dev Stats
vendor/knative.dev/test-infra/devstats

Copy link
Member

Choose a reason for hiding this comment

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

why is that ?

Copy link

@houshengbo houshengbo Nov 15, 2019

Choose a reason for hiding this comment

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

I would say not change this .gitignore by far.
The script under /hack/update-deps.sh is supposed to update the dependencies, and remove the useless files, including OWNERS. However, some packages may keep them, based on the configuration in Gopkg.toml. Let's keep the dependencies(with all the files) based on what is generated by /hack/update-deps.sh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing those.

if !set {
value = defaultValue
}
return value
Copy link
Member

Choose a reason for hiding this comment

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

👍

@matzew
Copy link
Member

matzew commented Nov 15, 2019

/ok-to-test

Also adding some extra OWNER files and test-infra/devstats from
vendor directory that shouldn't be commited
This commit let change test flags from outside. It can be done either by:

 * declaring environmental variables: `TEST_NAMESPACE`, and `TEST_RESOURCE`,
 * using `-ldflags "-X knative.dev/serving-operator/test.ServingOperatorNamespace=knative-serving"`

Changing those flags is needed to perform tests on RH operators.
@k4leung4
Copy link
Contributor

k4leung4 commented Dec 4, 2019

/approve
/lgtm

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, k4leung4

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

@knative-prow-robot knative-prow-robot merged commit 085e8a8 into knative:master Dec 4, 2019
cardil added a commit to cardil/serving-operator that referenced this pull request Dec 6, 2019
* Adding Goland to gitignore

Also adding some extra OWNER files and test-infra/devstats from
vendor directory that shouldn't be commited

* Letting flags be changed from outside

This commit let change test flags from outside. It can be done either by:

 * declaring environmental variables: `TEST_NAMESPACE`, and `TEST_RESOURCE`,
 * using `-ldflags "-X knative.dev/serving-operator/test.ServingOperatorNamespace=knative-serving"`

Changing those flags is needed to perform tests on RH operators.
cardil added a commit to cardil/serving-operator that referenced this pull request Dec 6, 2019
* Adding Goland to gitignore

Also adding some extra OWNER files and test-infra/devstats from
vendor directory that shouldn't be commited

* Letting flags be changed from outside

This commit let change test flags from outside. It can be done either by:

 * declaring environmental variables: `TEST_NAMESPACE`, and `TEST_RESOURCE`,
 * using `-ldflags "-X knative.dev/serving-operator/test.ServingOperatorNamespace=knative-serving"`

Changing those flags is needed to perform tests on RH operators.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants