refactor/lint: Add ignored shellcheck suggestions to an array #15170

pull koalaman wants to merge 1 commits into bitcoin:master from koalaman:master changing 1 files +25 −23

koalaman commented at 3:12 AM on January 15, 2019: contributor

By adding excluded shellcheck suggestions to an array, you can avoid the current duplication between command and comments. This ensures that they never go out of sync, makes it easier to add new ones, and improves the readability of related diffs.

refactor/lint: Add ignored suggestions to an array

This avoids duplicating the codes between command and comments.

cbd9091ed5

fanquake added the label Tests on Jan 15, 2019
Empact commented at 3:49 AM on January 15, 2019: member

Concept ACK
DrahtBot commented at 6:59 AM on January 15, 2019: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #15166 (qa: Ignore shellcheck warnings introduced in new versions by practicalswift)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
promag commented at 7:33 AM on January 15, 2019: member

utACK.

BTW, have you verified remaining linters?

in test/lint/lint-shell.sh:42 in cbd9091ed5

  60 | +    SC2086 # Double quote to prevent globbing and word splitting.
  61 | +    SC2116 # Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'.
  62 | +    SC2148 # Tips depend on target shell and yours is unknown. Add a shebang.
  63 | +    SC2162 # read without -r will mangle backslashes.
  64 | +    SC2166 # Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
  65 | +    SC2166 # Prefer [ p ] || [ q ] as [ p -o q ] is not well defined.

Empact commented at 7:35 AM on January 15, 2019:

nit: duplicate

in test/lint/lint-shell.sh:49 in cbd9091ed5

  67 | +    SC2206 # Quote to prevent word splitting, or split robustly with mapfile or read -a.
  68 | +    SC2207 # Prefer mapfile or read -a to split command output (or quote to avoid splitting).
  69 | +    SC2230 # which is non-standard. Use builtin 'command -v' instead.
  70 | +    SC2236 # Don't force -n instead of ! -z.
  71 | +)
  72 | +shellcheck -e "$(IFS=","; echo "${disabled[*]}")" \

Empact commented at 7:38 AM on January 15, 2019:

nit: --exclude= would be a bit more explicit/self-documenting

Empact commented at 7:43 AM on January 15, 2019: member

utACK https://github.com/bitcoin/bitcoin/pull/15170/commits/cbd9091ed5a76bb2e1e57cd0d8db035c15529341 - confirmed the same checks are disabled
practicalswift commented at 9:05 AM on January 15, 2019: contributor

utACK cbd9091ed5a76bb2e1e57cd0d8db035c15529341

Very nice to see a contribution from the shellcheck author. @koalaman, welcome as a contributor! Would be nice if you were willing to review the shellcheck related PR #15166 :-)
laanwj commented at 10:56 AM on January 16, 2019: member

Much better! I'd still prefer a list of cases to check, instead of those to not check, but this is at least DRY.

utACK cbd9091ed5a76bb2e1e57cd0d8db035c15529341
laanwj merged this on Jan 16, 2019
laanwj closed this on Jan 16, 2019
laanwj referenced this in commit 16c4a5373b on Jan 16, 2019
laanwj referenced this in commit 9bad1e0b22 on Jan 21, 2019
zkbot referenced this in commit 43ac2062f9 on Oct 28, 2020
zkbot referenced this in commit 84a5830aaa on Nov 9, 2020
PastaPastaPasta referenced this in commit 6b29f07e5c on Jun 26, 2021
PastaPastaPasta referenced this in commit a54f338783 on Jun 28, 2021
Munkybooty referenced this in commit 6795d46e4f on Aug 21, 2021
Munkybooty referenced this in commit 43c6128944 on Aug 23, 2021
Munkybooty referenced this in commit d936c1420c on Aug 24, 2021
Munkybooty referenced this in commit 31e68b8988 on Aug 24, 2021
Munkybooty referenced this in commit 8c7735da6b on Aug 24, 2021
UdjinM6 referenced this in commit 34614f8cff on Aug 24, 2021
Munkybooty referenced this in commit 846473aed2 on Aug 24, 2021
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#15219 lint: Enable python linters via an array

refactor/lint: Add ignored shellcheck suggestions to an array #15170

Conflicts