Skip to content

chore(ci): run linter on proto and grpc modules when librarian.yaml is updated #14004

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

Open
zhumin8 wants to merge 1 commit into
base: main
Choose a base branch
from
+9 −3
Open

chore(ci): run linter on proto and grpc modules when librarian.yaml is updated #14004

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions .kokoro/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,12 @@ case ${JOB_TYPE} in
changed_file_list=$(git diff --name-only "${BASE_SHA}" "${HEAD_SHA}" --relative)
echo "${changed_file_list}"

has_librarian_change="false"
if grep -q "librarian.yaml" <<< "${changed_file_list}"; then

@gemini-code-assist gemini-code-assist Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using grep -q "librarian.yaml" can lead to false positives if there are other files containing librarian.yaml as a substring (e.g., not_librarian.yaml or some_librarian.yaml). To ensure we only match files named exactly librarian.yaml (either at the root or in a subdirectory), use a regular expression that matches the start of a line or a slash, followed by librarian.yaml at the end of the line.

Suggested change
if grep -q "librarian.yaml" <<< "${changed_file_list}"; then
if grep -qE "(^|/)librarian\.yaml$" <<< "${changed_file_list}"; then

has_librarian_change="true"
echo "librarian.yaml modified; proto-* and grpc-* modules will be included in lint check."
fi

has_code_change="false"

while IFS= read -r changed_file; do
Expand Down Expand Up @@ -254,13 +260,13 @@ case ${JOB_TYPE} in
# Filter out directories not participating in the default formatting reactor:
# - samples are handwritten by developers
# - benchmarks are handwritten by developers
# - proto-*/grpc-* are generated code and should use the compiler format
# - proto-*/grpc-* are generated code (skipped unless librarian.yaml is updated)
# - *-bom/parents are POM-only and contain no Java source
if [[ "${dir}" != *"samples"* ]] && \
[[ "${dir}" != *"java-showcase"* ]] && \
[[ "$(basename "${dir}")" != *"benchmark"* ]] && \
[[ "$(basename "${dir}")" != "proto-google-"* ]] && \
[[ "$(basename "${dir}")" != "grpc-google-"* ]] && \
{ [ "${has_librarian_change}" == "true" ] || [[ "$(basename "${dir}")" != "proto-google-"* ]]; } && \
{ [ "${has_librarian_change}" == "true" ] || [[ "$(basename "${dir}")" != "grpc-google-"* ]]; } && \
Comment on lines +268 to +269

@gemini-code-assist gemini-code-assist Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using { [ ... ] || [[ ... ]]; } is complex and mixes different test syntaxes ([ and [[) along with command grouping { ... }. Since Bash's [[ ... ]] natively supports the || operator, you can simplify these conditions to be much more readable and idiomatic.

Suggested change
{ [ "${has_librarian_change}" == "true" ] || [[ "$(basename "${dir}")" != "proto-google-"* ]]; } && \
{ [ "${has_librarian_change}" == "true" ] || [[ "$(basename "${dir}")" != "grpc-google-"* ]]; } && \
[[ "${has_librarian_change}" == "true" || "$(basename "${dir}")" != "proto-google-"* ]] && \
[[ "${has_librarian_change}" == "true" || "$(basename "${dir}")" != "grpc-google-"* ]] && \

[[ "$(basename "${dir}")" != *"-bom" ]] && \
[[ "$(basename "${dir}")" != "google-cloud-pom-parent" ]] && \
[[ "$(basename "${dir}")" != "dependency-analyzer" ]] && \
Expand Down
Loading