Skip to content

CONTRIBUTING: Drop stale structure-check references #248

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

Merged

Conversation

wking
Copy link
Member

@wking wking commented Sep 13, 2018

They haven't worked since d61abd4 (coreos/tectonic-installer#3137). Recommend out new hack/* scripts instead.

I've also included two commits to make those hack scripts easier for devs to use. Details on those changes in their commit messages.

Setting --workdir and passing ${@} through means:

  $ ./hack/shellcheck.sh

will show any errors with relative paths that will still work outside
the container.  For example:

  ./tests/smoke/.build/external/go_sdk/src/syscall/mkall.sh:278:7: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]

instead of:

  /workdir/tests/smoke/.build/external/go_sdk/src/syscall/mkall.sh:278:7: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]

Adding prune rules for vendor and .build under tests/smoke avoids
complaining about that file and others over which are maintained
upstream or elsewhere.
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 13, 2018
This way they can use:

  $ hack/tf-fmt.sh -list -check

to update their local Terraform (vs. just checking if it's correctly
formatted).  I've also dropped the read-only mount option to support
this use case.

I use set [1] to inject default args if the caller set no positional
args themselves [2], so the test config in openshift/release continues
to work.

[1]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#set
[2]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_05_02
@wking wking force-pushed the drop-structure-check-references branch from 9520115 to 9108c3a Compare September 13, 2018 18:08
They haven't worked since d61abd4 (*: cleanup bazel rules,
2018-03-26, coreos/tectonic-installer#3137).  Recommend out new hack/*
scripts instead.
@wking
Copy link
Member Author

wking commented Sep 13, 2018

The e2e-aws and e2e-aws-smoke errors were:

2018/09/13 18:12:33 Build installer-bazel failed, printing logs:
Pulling image "docker-registry.default.svc:5000/ci-op-y867v638/pipeline@sha256:cb978ab18c464b7e0ea727a8fb66502bcb93c2a162f725d79441cd08a0818ef7" ...
error: error pulling image docker-registry.default.svc:5000/ci-op-y867v638/pipeline@sha256:cb978ab18c464b7e0ea727a8fb66502bcb93c2a162f725d79441cd08a0818ef7: Cannot overwrite digest sha256:cb978ab18c464b7e0ea727a8fb66502bcb93c2a162f725d79441cd08a0818ef7

That seems to be a Docker issue with a fix in-flight upstream (moby/moby#37781), but the patch may still need percolating out into various Red Hat consumers (rhbz#1625457 rhbz#1628262).

@wking
Copy link
Member Author

wking commented Sep 13, 2018

/retest

@crawford
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 13, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, wking

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-merge-robot openshift-merge-robot merged commit 2457075 into openshift:master Sep 14, 2018
@wking wking deleted the drop-structure-check-references branch September 14, 2018 05:33
wking added a commit to wking/openshift-installer that referenced this pull request Sep 17, 2018
We want to stuff these in when the user gave us no arguments, not
clobber arguments when the user passed some in :p.  Fixes a bug from
4b9cbdd (hack/tf-fmt: Allow callers to pass positional args,
2018-09-13, openshift#248).
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/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