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.

Sign up for GitHub

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

Jump to bottom

fix shellcheck pass in sample-apiserver #73210

Merged
merged 1 commit into from
Jan 30, 2019
Merged

fix shellcheck pass in sample-apiserver #73210

merged 1 commit into from
Jan 30, 2019

Conversation

danielqsj
Copy link
Contributor

@danielqsj danielqsj commented Jan 23, 2019 •
edited

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

fix shellcheck failures in staging/src/k8s.io/sample-apiserver

Which issue(s) this PR fixes:

work towards #72956

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 23, 2019
@danielqsj
Copy link
Contributor Author

/cc @BenTheElder

@danielqsj
Copy link
Contributor Author

/retest

BenTheElder
BenTheElder approved these changes Jan 24, 2019
View reviewed changes
Copy link
Member

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

/lgtm
thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 24, 2019
@fedebongio
Copy link
Contributor

/assign @logicalhan

@danielqsj
Copy link
Contributor Author

@deads2k if you have time, can you help review this? Thanks

@sttts sttts removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2019
@danielqsj
Copy link
Contributor Author

@sttts fixed, PTAL

@danielqsj danielqsj force-pushed the sample branch 2 times, most recently from d6f4323 to e2e4373 Compare January 29, 2019 01:45
@danielqsj
Copy link
Contributor Author

/cc @fejta

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 29, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 30, 2019
BenTheElder
BenTheElder reviewed Jan 30, 2019
View reviewed changes
staging/src/k8s.io/sample-apiserver/hack/update-codegen.sh Outdated

# generate the code with:
# --output-base because this script should also be able to run inside the vendor dir of
# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
# instead of the $GOPATH directly. For normal projects this can be dropped.
${CODEGEN_PKG}/generate-groups.sh all \
"${CODEGEN_PKG}"/generate-groups.sh all \
Copy link
Member

Choose a reason for hiding this comment

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

please quote the whole string

Suggested change
"${CODEGEN_PKG}"/generate-groups.sh all \
"${CODEGEN_PKG}/generate-groups.sh" all \

staging/src/k8s.io/sample-apiserver/hack/update-codegen.sh Outdated

${CODEGEN_PKG}/generate-internal-groups.sh "deepcopy,defaulter,conversion" \
"${CODEGEN_PKG}"/generate-internal-groups.sh "deepcopy,defaulter,conversion" \
Copy link
Member

Choose a reason for hiding this comment

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

here too:

Suggested change
"${CODEGEN_PKG}"/generate-internal-groups.sh "deepcopy,defaulter,conversion" \
"${CODEGEN_PKG}/generate-internal-groups.sh" "deepcopy,defaulter,conversion" \

staging/src/k8s.io/sample-apiserver/hack/update-codegen.sh Outdated
--output-base "$(dirname ${BASH_SOURCE})/../../.." \
--go-header-file ${SCRIPT_ROOT}/hack/boilerplate.go.txt
--output-base "$(dirname "${BASH_SOURCE[0]}")/../../.." \
--go-header-file "${SCRIPT_ROOT}"/hack/boilerplate.go.txt
Copy link
Member

Choose a reason for hiding this comment

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

and here:

Suggested change
--go-header-file "${SCRIPT_ROOT}"/hack/boilerplate.go.txt
--go-header-file "${SCRIPT_ROOT}/hack/boilerplate.go.txt"

@danielqsj
Copy link
Contributor Author

@BenTheElder fixed, thanks :)

BenTheElder
BenTheElder approved these changes Jan 30, 2019
View reviewed changes
Copy link
Member

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2019
@sttts
Copy link
Contributor

sttts commented Jan 30, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danielqsj, sttts

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 30, 2019
@sttts
Copy link
Contributor

sttts commented Jan 30, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 529785e into kubernetes:master Jan 30, 2019
@k8s-ci-robot
Copy link
Contributor

@danielqsj: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 0b4397f link /test pull-kubernetes-e2e-kops-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@BenTheElder BenTheElder mentioned this pull request Feb 11, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

@BenTheElder BenTheElder BenTheElder approved these changes

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

@vishh vishh Awaiting requested review from vishh

@fejta fejta Awaiting requested review from fejta

Assignees

@logicalhan logicalhan

@BenTheElder BenTheElder

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@danielqsj @fedebongio @sttts @k8s-ci-robot @BenTheElder @logicalhan