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.

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

hack: update verify-code-patterns.sh #221

Merged
merged 1 commit into from
Jun 27, 2019

Conversation

ahmetb
Copy link
Member

@ahmetb ahmetb commented Jun 24, 2019

This fixes an issue where sometimes bash returns 1 instead of 123
in the exitcode of the pipe. This simplifies the find|xargs|grep with
just grep.

Fixes #217
/assign @corneliusweig

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 24, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmetb

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 24, 2019
@kubernetes-sigs kubernetes-sigs deleted a comment from k8s-ci-robot Jun 24, 2019
Copy link
Contributor

@corneliusweig corneliusweig left a comment

Choose a reason for hiding this comment

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

The output looks great and it's much simpler like that!

One of the options needs to be adjusted (at least on my system).

hack/verify-code-patterns.sh Outdated Show resolved Hide resolved
@ahmetb
Copy link
Member Author

ahmetb commented Jun 25, 2019

One of the options needs to be adjusted (at least on my system).

Does it work if I say --exclude 'vendor/*'? I hope it's not BSD grep vs GNU grep difference?

@corneliusweig
Copy link
Contributor

corneliusweig commented Jun 25, 2019

No, --exclude 'vendor/*' or similar does unfortunately not work. My man page says:

--exclude=GLOB
Skip any command-line file with a name suffix that matches the pattern GLOB, using
wildcard matching; a name suffix is either the whole name, or any suffix starting
after a / and before a +non-/. When searching recursively, skip any subfile whose
base name matches GLOB; the base name is the part after the last /. A pattern can
use *, ?, and [...] as wildcards, and \ to quote a wildcard or backslash
character literally.

So it's clearly limited to the basename.


For completeness:

--exclude-dir=GLOB
Skip any command-line directory with a name suffix that matches the pattern GLOB.
When searching recursively, skip any subdirectory whose base name matches GLOB.
Ignore any redundant trailing slashes in GLOB.

@ahmetb
Copy link
Member Author

ahmetb commented Jun 25, 2019

Yeah makes sense. I'll fix.

This fixes an issue where sometimes bash returns 1 instead of 123
in the exitcode of the pipe. This simplifies the find|xargs|grep with
just grep.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@4b0255c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #221   +/-   ##
=========================================
  Coverage          ?   53.78%           
=========================================
  Files             ?       14           
  Lines             ?      701           
  Branches          ?        0           
=========================================
  Hits              ?      377           
  Misses            ?      273           
  Partials          ?       51

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b0255c...8754518. Read the comment docs.

@ahmetb ahmetb merged commit 4e47fbc into kubernetes-sigs:master Jun 27, 2019
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hack/verify-code-patterns.sh: fails locally
4 participants