Skip to content

Conversation

@sallyom
Copy link
Contributor

@sallyom sallyom commented Aug 1, 2018

Before creating your PR, please make sure to add the appropriate GitHub label; i.e. run-smoke-tests. For more details see
tests/README.md.

(In case you don't have permissions to add labels, please ask a
Maintainer.)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 1, 2018
@coreosbot
Copy link

Can one of the admins verify this patch?

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 1, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sallyom
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: crawford

If they are not already assigned, you can assign the PR to them by writing /assign @crawford in a comment when ready.

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

Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me. I'm in favor of tackling this with a series of small PRs instead of one big Bazel -> Make PR, so I think we can go ahead and land this without waiting for it to grow larger :p.

- script: "chmod 0777 $PWD && docker run -v $PWD:$PWD:rw -w $PWD $BAZEL_IMG bazel test terraform_fmt --test_output=all"
name: Terraform tests
- script: "chmod 0777 $PWD && docker run -v $PWD:$PWD:rw -w $PWD $BAZEL_IMG bazel test installer:cli_units --test_output=all"
name: Installer unit tests
Copy link
Member

Choose a reason for hiding this comment

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

I'd leave these in for now to make sure we're covered until Prow picks them up (the Terraform linter is in flight with openshift/release#1124).


#src_files=$(shell find installer -type f -name "*.go")

test:
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer a more specific name like test-go or some such. But maybe test is a standard name for Go-only tests throughout OpenShift projects or something.

@sallyom
Copy link
Contributor Author

sallyom commented Aug 1, 2018

closing for now, in favor of small individual scripts for prow, will open a new PR w/ that in mind

@sallyom sallyom closed this Aug 1, 2018
@wking wking mentioned this pull request Feb 26, 2019
stbenjam pushed a commit to stbenjam/installer that referenced this pull request Feb 10, 2021
Bug 1913716: Use existing libraries instead of rolling our own
clnperez added a commit to clnperez/installer that referenced this pull request Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants