Skip to content

hack: add verify script for boilerplate and yamllint check#1765

Merged
k8s-ci-robot merged 2 commits intokubernetes:mainfrom
nikhita:verify-script
Mar 9, 2021
Merged

hack: add verify script for boilerplate and yamllint check#1765
k8s-ci-robot merged 2 commits intokubernetes:mainfrom
nikhita:verify-script

Conversation

@nikhita
Copy link
Member

@nikhita nikhita commented Mar 7, 2021

Ref: "ideas of follow up" in kubernetes/test-infra#20956

This PR:

  • Updates existing verify-yamllint.sh script to install yamllint if
    it does not exist.

  • Adds verify-boilerplate.sh script for verifying boilerplate.
    This is derived from the script in the enhancements repo.

  • Adds verify.sh script to test all verify scripts at once. This would
    be useful for adding future verify scripts.

  • Fixes boilerplate headers to make the script pass

/assign @spiffxp

nikhita added 2 commits March 7, 2021 21:17
- Update existing `verify-yamllint.sh` script to install yamllint if
it does not exist.

- Add `verify-boilerplate.sh` script for verifying boilerplate.

- Add `verify.sh` script to test all verify scripts at once. This would
be useful for adding future verify scripts.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 7, 2021
@k8s-ci-robot k8s-ci-robot requested review from fejta and wongma7 March 7, 2021 16:00
@k8s-ci-robot k8s-ci-robot added area/artifacts Issues or PRs related to the hosting of release artifacts for subprojects area/dns DNS records for k8s.io, kubernetes.io, k8s.dev, etc., code in dns/ area/audit Audit of project resources, audit followup issues, code in audit/ area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters sig/testing Categorizes an issue or PR as relevant to SIG Testing. wg/k8s-infra size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 7, 2021
@spiffxp
Copy link
Contributor

spiffxp commented Mar 7, 2021

/cc @ameukam

@k8s-ci-robot k8s-ci-robot requested a review from ameukam March 7, 2021 17:27
echo >/dev/stderr "ERROR: yamllint not found - please install with: ${pip} install -r ${pip_requirements}"
exit 1
echo "yamllint not found - installing with: ${pip} install -r ${pip_requirements}"
${pip} install -r ${pip_requirements}
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to have a comment saying we assume pip3 is already installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point, I think we can address in followup. Other scripts in this repo assume the presence of python3, which implies pip3. Not sure if they verify their assumptions either

Copy link
Contributor

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

set -o nounset
set -o pipefail

KUBE_ROOT=$(dirname "${BASH_SOURCE}")/..
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit for followup: I think I would rather use REPO_ROOT or something not so tied to k/k script conventions. Repo root is what I saw in some test-infra scripts.

I don't feel strongly about this but it would be good to standardize across the scripts we have in this repo. This should at least use BASH_SOURCE[0] as a followup

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 8, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhita, spiffxp

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 8, 2021
@k8s-ci-robot k8s-ci-robot merged commit 089f728 into kubernetes:main Mar 9, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Mar 9, 2021
@nikhita nikhita deleted the verify-script branch March 9, 2021 11:51
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. area/artifacts Issues or PRs related to the hosting of release artifacts for subprojects area/audit Audit of project resources, audit followup issues, code in audit/ area/dns DNS records for k8s.io, kubernetes.io, k8s.dev, etc., code in dns/ area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. 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.

4 participants