#1841 Skip TAGS_MISMATCH check if no English original exists

Open
fbn_bmns wants to merge 1 commits from fbn_bmns/fsfe-website:fix/skip_tags_mismatch_check_no_en into master

Skip TAGS_MISMATCH check if no English original file exists.

Fixes #1821.

Skip TAGS_MISMATCH check if no English original file exists. Fixes #1821.
fbn_bmns added 1 commit 2 months ago
max.mehl requested changes 1 month ago
max.mehl left a comment

Thank you for the fix!

A few suggestions pending though. And may I please ask you to retain the current indentation format of 2 spaces? Thanks!

base=$(echo "${f}" | sed -E "s/\.[a-z][a-z]\.${ext}//")
# exit TAGS_MISMATCH check if no english original exists
if [[ ! -e "$base.en.$ext" ]]; then
echo "No english original found for $f, skipping TAGS_MISMATCH check.."
max.mehl commented 1 month ago

Please do not echo. The hook should only output if something is wrong

Please do not echo. The hook should only output if something is wrong
# exit TAGS_MISMATCH check if no english original exists
if [[ ! -e "$base.en.$ext" ]]; then
echo "No english original found for $f, skipping TAGS_MISMATCH check.."
break
max.mehl commented 1 month ago

IMHO break is not what we want. The whole for loop would halt, and therefore the next checks aborted.

I'd rather recommend to do it like this:

if [[ -e "$base.en.$ext" ]]; then
  # the existing checks here, up to line 142 in the current file
fi
IMHO `break` is not what we want. The whole `for` loop would halt, and therefore the next checks aborted. I'd rather recommend to do it like this: ```sh if [[ -e "$base.en.$ext" ]]; then # the existing checks here, up to line 142 in the current file fi ```

Reviewers

max.mehl requested changes 1 month ago
All checks were successful
continuous-integration/drone/pr Build is passing
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.