ci: Move spellchecks to format_pre #17028
Conversation
|
im trying to understand why we have 2 sets of spellchecking afaict also |
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
phlax
changed the title
|
on the basis that i wasnt able to trigger the misspell spelling check, and that the fix doesnt work non-interactively for the pedantic spelling (as discussed offline) i have removed the first and set the latter to check only, as well as moving the spell checking to the cc @zuercher |
Signed-off-by: Ryan Northey <[email protected]>
| @@ -419,22 +418,6 @@ elif [[ "$CI_TARGET" == "check_format" ]]; then | |||
| "${ENVOY_SRCDIR}"/tools/code_format/check_format.py check | |||
| BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS[*]}" "${ENVOY_SRCDIR}"/tools/proto_format/proto_format.sh check --test | |||
| exit 0 | |||
| elif [[ "$CI_TARGET" == "check_spelling" ]]; then | |||
There was a problem hiding this comment.
hm, do you think we should keep these around for folks who are used to running things this way, or only want to do spelling not all pre checks?
We can also remove it now and add it back if anyone complains but I have these aliased so I'd mildly prefer to keep but I can totally update my aliases too :-)
There was a problem hiding this comment.
i think we need to remove these - i think the do_ci.sh switchboard is a bit of an anti pattern - the related readme has been updated
| @@ -58,6 +58,9 @@ bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/code_format:python_check -- --diff | |||
| CURRENT=extensions | |||
| bazel run "${BAZEL_BUILD_OPTIONS[@]}" //tools/extensions:extensions_check | |||
|
|
|||
| CURRENT=spelling | |||
| "${ENVOY_SRCDIR}"/tools/spelling/check_spelling_pedantic.py --mark check | |||
There was a problem hiding this comment.
why delete the other spell check? I thought they caught different things?
There was a problem hiding this comment.
afaict one is not catching anything
when this was discussed in the maintainer channel it was ~agreed to remove
There was a problem hiding this comment.
ah I must have been out
|
It looks like this PR broke some scripts (well shell aliases) I was using to check the spelling. (though I didn't notice immediately). Perhaps in the future it would be a good idea to announce workflow changes like this so developer know what they need to do? |
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey [email protected]
Commit Message: ci: Move spellchecks to format_pre
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]