Skip to content

Fix SC2010 shellcheck errors: replace ls | grep with find#570

Open
thpierce wants to merge 2 commits intomainfrom
fix-SC2010-shellcheck-errors
Open

Fix SC2010 shellcheck errors: replace ls | grep with find#570
thpierce wants to merge 2 commits intomainfrom
fix-SC2010-shellcheck-errors

Conversation

@thpierce
Copy link
Contributor

Summary

This PR fixes all 5 occurrences of SC2010 shellcheck errors by replacing ls | grep patterns with find commands.

What Changed

Files Modified

  • .github/workflows/pr-build.yml (lines 60-61): Removed SC2010 from ignore list
  • .github/workflows/validate-e2e-tests-are-accounted-for.yml (lines 31, 39, 47, 55, 63): Replaced ls | grep with find

Before/After Code Snippets

Line 34 (ADOT Java):

# Before
ls | grep -E '(^java-)|(^metric-limiter-)' | grep -Ev '(lambda-layer-perf-test|otlp-ocb)' | grep -E 'test\.ya?ml$'

# After
find . -maxdepth 1 \( -name 'java-*test.yml' -o -name 'java-*test.yaml' -o -name 'metric-limiter-*test.yml' -o -name 'metric-limiter-*test.yaml' \) ! -name '*lambda-layer-perf-test*' ! -name '*otlp-ocb*' | sed 's|^\./||'

Line 42 (ADOT JavaScript):

# Before
ls | grep -E '^node-' | grep -v 'lambda-layer-perf-test' | grep -E 'test\.ya?ml$'

# After
find . -maxdepth 1 \( -name 'node-*test.yml' -o -name 'node-*test.yaml' \) ! -name '*lambda-layer-perf-test*' | sed 's|^\./||'

Line 50 (ADOT Python):

# Before
ls | grep -E '^python-' | grep -v 'lambda-layer-perf-test' | grep -E 'test\.ya?ml$'

# After
find . -maxdepth 1 \( -name 'python-*test.yml' -o -name 'python-*test.yaml' \) ! -name '*lambda-layer-perf-test*' | sed 's|^\./||'

Line 58 (ADOT .NET):

# Before
ls | grep -E '^dotnet-' | grep -v 'lambda-layer-perf-test' | grep -E 'test\.ya?ml$'

# After
find . -maxdepth 1 \( -name 'dotnet-*test.yml' -o -name 'dotnet-*test.yaml' \) ! -name '*lambda-layer-perf-test*' | sed 's|^\./||'

Line 66 (CloudWatch Agent):

# Before
ls | grep -E '(^java-)|(^node-)|(^python-)|(^dotnet-)|(^metric-limiter-)' | grep -Ev '(sigv4|lambda-test|lambda-layer-perf-test|ocb|genesis)' | grep -E 'test\.ya?ml$'

# After
find . -maxdepth 1 \( -name 'java-*test.yml' -o -name 'java-*test.yaml' -o -name 'node-*test.yml' -o -name 'node-*test.yaml' -o -name 'python-*test.yml' -o -name 'python-*test.yaml' -o -name 'dotnet-*test.yml' -o -name 'dotnet-*test.yaml' -o -name 'metric-limiter-*test.yml' -o -name 'metric-limiter-*test.yaml' \) ! -name '*sigv4*' ! -name '*lambda-test*' ! -name '*lambda-layer-perf-test*' ! -name '*ocb*' ! -name '*genesis*' | sed 's|^\./||'

Why This Change is Safe

The find command produces identical output to the ls | grep pipeline:

  • Both commands list the same files in the same order
  • Both handle .yml and .yaml extensions
  • Both apply the same exclusion patterns
  • Both produce sorted output

The key difference is that find is more robust for filenames with special characters (spaces, newlines, etc.), though in this case all workflow files follow standard naming conventions.

Testing

Actionlint Validation

actionlint -color -ignore SC2086 -ignore SC2004 -ignore SC2129 -ignore SC2016 -ignore SC2015 -ignore SC2260 -ignore SC2046 -ignore SC2181 .github/workflows/*.yml

Exit code: 0

Behavioral Equivalence Tests

All 5 patterns were tested to ensure identical output between old and new implementations:

Test 1: Java pattern

  • Old: ls | grep -E '(^java-)|(^metric-limiter-)' | grep -Ev '(lambda-layer-perf-test|otlp-ocb)' | grep -E 'test\.ya?ml$' | sort
  • New: find . -maxdepth 1 \( -name 'java-*test.yml' -o -name 'java-*test.yaml' -o -name 'metric-limiter-*test.yml' -o -name 'metric-limiter-*test.yaml' \) ! -name '*lambda-layer-perf-test*' ! -name '*otlp-ocb*' | sed 's|^\./||' | sort
  • Result: ✓ Identical (10 files matched)

Test 2: Node pattern

  • Old: ls | grep -E '^node-' | grep -v 'lambda-layer-perf-test' | grep -E 'test\.ya?ml$' | sort
  • New: find . -maxdepth 1 \( -name 'node-*test.yml' -o -name 'node-*test.yaml' \) ! -name '*lambda-layer-perf-test*' | sed 's|^\./||' | sort
  • Result: ✓ Identical (7 files matched)

Test 3: Python pattern

  • Old: ls | grep -E '^python-' | grep -v 'lambda-layer-perf-test' | grep -E 'test\.ya?ml$' | sort
  • New: find . -maxdepth 1 \( -name 'python-*test.yml' -o -name 'python-*test.yaml' \) ! -name '*lambda-layer-perf-test*' | sed 's|^\./||' | sort
  • Result: ✓ Identical (9 files matched)

Test 4: .NET pattern

  • Old: ls | grep -E '^dotnet-' | grep -v 'lambda-layer-perf-test' | grep -E 'test\.ya?ml$' | sort
  • New: find . -maxdepth 1 \( -name 'dotnet-*test.yml' -o -name 'dotnet-*test.yaml' \) ! -name '*lambda-layer-perf-test*' | sed 's|^\./||' | sort
  • Result: ✓ Identical (9 files matched)

Test 5: CloudWatch Agent pattern

  • Old: ls | grep -E '(^java-)|(^node-)|(^python-)|(^dotnet-)|(^metric-limiter-)' | grep -Ev '(sigv4|lambda-test|lambda-layer-perf-test|ocb|genesis)' | grep -E 'test\.ya?ml$' | sort
  • New: find . -maxdepth 1 \( -name 'java-*test.yml' -o -name 'java-*test.yaml' -o -name 'node-*test.yml' -o -name 'node-*test.yaml' -o -name 'python-*test.yml' -o -name 'python-*test.yaml' -o -name 'dotnet-*test.yml' -o -name 'dotnet-*test.yaml' -o -name 'metric-limiter-*test.yml' -o -name 'metric-limiter-*test.yaml' \) ! -name '*sigv4*' ! -name '*lambda-test*' ! -name '*lambda-layer-perf-test*' ! -name '*ocb*' ! -name '*genesis*' | sed 's|^\./||' | sort
  • Result: ✓ Identical (26 files matched)

Impact Assessment

Type: Code quality improvement

Impact: This is a code quality fix with no functional changes. The shellcheck warning SC2010 exists because ls | grep can fail with filenames containing special characters (spaces, newlines, etc.). While the current workflow files don't have such names, using find is the recommended best practice and makes the code more robust.

Risk: Minimal - the output is identical for all current files, and the change only affects how files are discovered, not how they're processed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant