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

Add verify-spelling.sh for English words. #547

Merged
merged 2 commits into from
May 5, 2023
Merged

Add verify-spelling.sh for English words. #547

merged 2 commits into from
May 5, 2023

Conversation

yanggangtony
Copy link
Member

@yanggangtony yanggangtony commented Apr 25, 2023

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add verify-spelling.sh to check english typos...

Ignore .golangci.yaml,

./.golangci.yaml:22:6: "importas" is a misspelling of "imports"
./.golangci.yaml:50:2: "importas" is a misspelling of "imports"

because importas is the package name.
golinters importas typos

Which issue(s) this PR fixes:

Fixes #545

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 25, 2023
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 25, 2023
@netlify
Copy link

netlify bot commented Apr 25, 2023

Deploy Preview for k8s-kwok canceled.

Name Link
🔨 Latest commit 2edbdd2
🔍 Latest deploy log https://app.netlify.com/sites/k8s-kwok/deploys/645478fbb67a2b0008ecc03b

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 25, 2023
echo "Checking spelling..."
ERROR_LOG="${TMP_DIR}/errors.log"

find . -type f | grep -v .golangci.yaml | grep -v vendor/ | xargs misspell >"${ERROR_LOG}"
Copy link
Member

Choose a reason for hiding this comment

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

We should have a list of allowed words, not the whole document excluded.

Copy link
Member Author

@yanggangtony yanggangtony Apr 26, 2023

Choose a reason for hiding this comment

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

@wzshiming
thanks for review.

you means like the code below

skipping_file="${KUBE_ROOT}/scripts/.spelling_failures"
failing_packages=$(sed "s| | -e |g" "${skipping_file}")

And we add the .golangci.yaml and vendor/ to .spelling_failures files?

Actually i used did it like that, but error shows,
So i update the code ..😂😂

If that , i will try again and test ..

Copy link
Member

@wzshiming wzshiming Apr 26, 2023

Choose a reason for hiding this comment

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

For more tools details see https://github.com/client9/misspell
Refer #514 to add verify-spelling.sh and update-spelling.sh

with -i $(sed <"${spelling_words}" "s| |,|g")

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks .
I will do it later.

hack/verify-all.sh Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 4, 2023
hack/update-spelling.sh Outdated Show resolved Hide resolved
hack/allowd_spelling_words Outdated Show resolved Hide resolved

function check() {
#NOTE we use go install refer the go site :https://go.dev/doc/go-get-install-deprecation
go install github.com/client9/misspell/cmd/misspell@${TOOL_VERSION}
Copy link
Member

Choose a reason for hiding this comment

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

@yanggangtony
Copy link
Member Author

So fast for review...😀😀

@yanggangtony
Copy link
Member Author

yanggangtony commented May 4, 2023

vendor dir is not exclude..
i will test it again.
wait a moment for rereviw.

@yanggangtony
Copy link
Member Author

@wzshiming
the ci passd.
would you please review again?

Comment on lines 16 to 27
##########
# This script verifies mispellings in location. Today it only supports
# verifying English locale but can be modified in a future to support
# other locales also
# You need to run this script inside the root directory of "website" git repo.
#
# Syntax: verify-spelling.sh LOCALE
# Example: verify-spelling.sh en
# If no locale is passed, it will assume "en"
#
# Requirements:
# - go v1.14 or superior version
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
##########
# This script verifies mispellings in location. Today it only supports
# verifying English locale but can be modified in a future to support
# other locales also
# You need to run this script inside the root directory of "website" git repo.
#
# Syntax: verify-spelling.sh LOCALE
# Example: verify-spelling.sh en
# If no locale is passed, it will assume "en"
#
# Requirements:
# - go v1.14 or superior version

@@ -0,0 +1,42 @@
#!/usr/bin/env bash

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Comment on lines 22 to 23
# cd to the root path
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# cd to the root path
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)"
ROOT_DIR="$(realpath "$(dirname "${BASH_SOURCE[0]}")"/..)"

fi

function check() {
cd "${ROOT}"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cd "${ROOT}"

Comment on lines 28 to 34

# cleanup
exitHandler() (
echo "Cleaning up..."
rm -rf "${TMP_DIR}"
)
trap exitHandler EXIT
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# cleanup
exitHandler() (
echo "Cleaning up..."
rm -rf "${TMP_DIR}"
)
trap exitHandler EXIT

Comment on lines 69 to 72
if [[ "${VERIFY_SHELL_SPELLING:-true}" == "true" ]]; then
echo "[*] Verifying spelling..."
"${ROOT_DIR}"/hack/verify-spelling.sh || failed+=(shell-spelling)
fi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ "${VERIFY_SHELL_SPELLING:-true}" == "true" ]]; then
echo "[*] Verifying spelling..."
"${ROOT_DIR}"/hack/verify-spelling.sh || failed+=(shell-spelling)
fi
if [[ "${VERIFY_SPELLING:-true}" == "true" ]]; then
echo "[*] Verifying spelling..."
"${ROOT_DIR}"/hack/verify-spelling.sh || failed+=(spelling)
fi

Comment on lines 54 to 57
if [[ "${UPDATE_SHELL_SPELLING:-true}" == "true" ]]; then
echo "[*] Update spelling..."
"${ROOT_DIR}"/hack/update-spelling.sh || failed+=(shell-spelling)
fi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ "${UPDATE_SHELL_SPELLING:-true}" == "true" ]]; then
echo "[*] Update spelling..."
"${ROOT_DIR}"/hack/update-spelling.sh || failed+=(shell-spelling)
fi
if [[ "${UPDATE_SPELLING:-true}" == "true" ]]; then
echo "[*] Update spelling..."
"${ROOT_DIR}"/hack/update-spelling.sh || failed+=(spelling)
fi

@@ -0,0 +1,42 @@
#!/usr/bin/env bash

# Copyright 2019 The Kubernetes Authors.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2019 The Kubernetes Authors.
# Copyright 2023 The Kubernetes Authors.

Comment on lines 39 to 40
elif command -v "${GOPATH}/bin/misspell"; then
COMMAND=("${GOPATH}/bin/misspell")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
elif command -v "${GOPATH}/bin/misspell"; then
COMMAND=("${GOPATH}/bin/misspell")
elif command -v "${ROOT_DIR}/bin/misspell"; then
COMMAND=("${ROOT_DIR}/bin/misspell")

Comment on lines 43 to 44
go install github.com/client9/misspell/cmd/misspell@${TOOL_VERSION}
COMMAND=(misspell)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
go install github.com/client9/misspell/cmd/misspell@${TOOL_VERSION}
COMMAND=(misspell)
GOBIN="${ROOT_DIR}/bin/" go install github.com/client9/misspell/cmd/misspell@${TOOL_VERSION}
COMMAND=("${ROOT_DIR}/bin/misspell")

@yanggangtony
Copy link
Member Author

Good suggestion.
Update later.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 4, 2023
Comment on lines 37 to 40
# check spelling
echo "Checking spelling..."
ignore="$(tr <"${allowd_spelling_words}" '\n' ',')"
"${COMMAND[@]}" -i "${ignore}" "$(find . -not \( -path ./vendor\* -o -path ./.git\* \))"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# check spelling
echo "Checking spelling..."
ignore="$(tr <"${allowd_spelling_words}" '\n' ',')"
"${COMMAND[@]}" -i "${ignore}" "$(find . -not \( -path ./vendor\* -o -path ./.git\* \))"
ignore="$(tr <"${allowd_spelling_words}" '\n' ',')"
"${COMMAND[@]}" -w -i "${ignore}" "$(find . -not \( -path ./vendor\* -o -path ./.git\* \))"

Copy link
Member Author

@yanggangtony yanggangtony May 4, 2023

Choose a reason for hiding this comment

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

Good suggestion.
with -w flag , So misspell can override the file.
And then git --no-pager diff --exit-code can check the diff of the file.

Will update it later.

elif command -v "${ROOT_DIR}/bin/misspell"; then
COMMAND=("${ROOT_DIR}/bin/misspell")
else
#NOTE we use go install refer the go site :https://go.dev/doc/go-get-install-deprecation
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#NOTE we use go install refer the go site :https://go.dev/doc/go-get-install-deprecation

TOOL_VERSION="v0.3.4"

ROOT_DIR="$(realpath "$(dirname "${BASH_SOURCE[0]}")"/..)"
allowd_spelling_words="${ROOT_DIR}/hack/allowd_spelling_words"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
allowd_spelling_words="${ROOT_DIR}/hack/allowd_spelling_words"
allowed_spelling_words="${ROOT_DIR}/hack/spelling.txt"

"${COMMAND[@]}" -i "${ignore}" "$(find . -not \( -path ./vendor\* -o -path ./.git\* \))"
}

cd "${ROOT_DIR}" && check || exit 1
Copy link
Member

@wzshiming wzshiming May 4, 2023

Choose a reason for hiding this comment

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

Suggested change
cd "${ROOT_DIR}" && check || exit 1
cd "${ROOT_DIR}"
update || exit 1

Comment on lines 36 to 37
ignore="$(tr <"${allowed_spelling_words}" '\n' ',')"
"${COMMAND[@]}" -w -i "${ignore}" "$(find . -not \( -path ./vendor\* -o -path ./.git\* \))"
Copy link
Member

@wzshiming wzshiming May 5, 2023

Choose a reason for hiding this comment

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

Suggested change
ignore="$(tr <"${allowed_spelling_words}" '\n' ',')"
"${COMMAND[@]}" -w -i "${ignore}" "$(find . -not \( -path ./vendor\* -o -path ./.git\* \))"
local ignore
ignore="$(tr <"${allowed_spelling_words}" '\n' ',')"
mapfile -t files < <(find . \( \
-iname "*.md" -o \
-iname "*.sh" -o \
-iname "*.go" -o \
-iname "*.tpl" -o \
-iname "*.yaml" -o \
-iname "*.yml" \
\) \
-not \( \
-path ./.git/\* -o \
-path ./vendor/\* -o \
-path ./demo/node_modules/\* -o \
-path ./site/themes/\* \
\))
"${COMMAND[@]}" -locale US -w -i "${ignore}" "${files[@]}"

Signed-off-by: yanggang <gang.yang@daocloud.io>
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 5, 2023
@yanggangtony
Copy link
Member Author

/retest

renderer: fmt.Sprintf("%s%s%s", c, msg, ctc.Reset),
width: stringWidth(msg),
}
}

var levelColour = map[string]colour{
var levelColour = map[string]color{
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var levelColour = map[string]color{
var levelColor = map[string]color{

Copy link
Member Author

Choose a reason for hiding this comment

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

good, why the misspell not found that....😂😂

Signed-off-by: yanggang <gang.yang@daocloud.io>
Copy link
Member

@wzshiming wzshiming 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
/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label May 5, 2023
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 5, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wzshiming, yanggangtony

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 May 5, 2023
@k8s-ci-robot k8s-ci-robot merged commit df42845 into kubernetes-sigs:main May 5, 2023
@yanggangtony yanggangtony deleted the feature-misspell branch May 5, 2023 05:43
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. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add verify-spelling.sh
3 participants