Skip to content

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

Merged
laanwj merged 1 commit intobitcoin:masterfrom
koalaman:master
Jan 16, 2019
Merged

refactor/lint: Add ignored shellcheck suggestions to an array#15170
laanwj merged 1 commit intobitcoin:masterfrom
koalaman:master

Conversation

@koalaman
Copy link
Contributor

@koalaman koalaman commented Jan 15, 2019

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.

@koalaman
refactor/lint: Add ignored suggestions to an array
cbd9091 
This avoids duplicating the codes between command and comments.
@fanquake fanquake added the Tests label Jan 15, 2019
@Empact
Copy link
Contributor

Empact commented Jan 15, 2019

Concept ACK

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 15, 2019

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
promag reviewed Jan 15, 2019
View reviewed changes
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

utACK.

BTW, have you verified remaining linters?

Empact
Empact reviewed Jan 15, 2019
View reviewed changes
test/lint/lint-shell.sh
SC2148 # Tips depend on target shell and yours is unknown. Add a shebang.
SC2162 # read without -r will mangle backslashes.
SC2166 # Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
SC2166 # Prefer [ p ] || [ q ] as [ p -o q ] is not well defined.
Copy link
Contributor

@Empact Empact Jan 15, 2019

Choose a reason for hiding this comment

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

nit: duplicate

Empact
Empact reviewed Jan 15, 2019
View reviewed changes
test/lint/lint-shell.sh
SC2230 # which is non-standard. Use builtin 'command -v' instead.
SC2236 # Don't force -n instead of ! -z.
)
shellcheck -e "$(IFS=","; echo "${disabled[*]}")" \
Copy link
Contributor

@Empact Empact Jan 15, 2019

Choose a reason for hiding this comment

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

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

@Empact
Copy link
Contributor

Empact commented Jan 15, 2019

utACK cbd9091 - confirmed the same checks are disabled

@practicalswift
Copy link
Contributor

practicalswift commented Jan 15, 2019

utACK cbd9091

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
Copy link
Member

laanwj commented Jan 16, 2019

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 cbd9091

@laanwj laanwj merged commit cbd9091 into bitcoin:master Jan 16, 2019
laanwj added a commit that referenced this pull request Jan 16, 2019
@laanwj
Merge #15170: refactor/lint: Add ignored shellcheck suggestions to an…
16c4a53 
… array

cbd9091 refactor/lint: Add ignored suggestions to an array (Vidar Holen)

Pull request description:

  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.

Tree-SHA512: 04afced1d27fda940cc5e61d7f9ed04507c8f7f7dfd0031c09898a599c6de93695923a80cb3d515a0f0bf728847592d8680c15ac2e376b48726c03ca744f13a5
@Empact Empact mentioned this pull request Jan 21, 2019
Merged
laanwj added a commit that referenced this pull request Jan 21, 2019
@laanwj
Merge #15219: lint: Enable python linters via an array
9bad1e0 
948d8f4 lint: Enable python linters via an array (Ben Woosley)

Pull request description:

  This assures consistent recording of the enabled linters.

  This applies the same fix as #15170 to lint-python.sh

Tree-SHA512: 6d03f919e86e7c2465475c88b25dd84391282bcc11728078024daf0432a7dccddf9e4a2cdae35d6ef374971cb4e12f0fa21b58f757e25f2fe7c12ceb4f4b2c57
@hebasto hebasto mentioned this pull request Jul 2, 2019
Merged
@str4d str4d mentioned this pull request Oct 27, 2020
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Oct 28, 2020
@zkbot
Auto merge of #4829 - str4d:backport-lints, r=<try>
43ac206 
Backport useful lints from upstream

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6892
- bitcoin/bitcoin#11151
- bitcoin/bitcoin#11300
- bitcoin/bitcoin@96d91b7
- bitcoin/bitcoin#12097
- bitcoin/bitcoin#12098
- bitcoin/bitcoin#12442
- bitcoin/bitcoin#12572
- bitcoin/bitcoin#12757
- bitcoin/bitcoin#11878
- bitcoin/bitcoin#12933
- bitcoin/bitcoin#12871
- bitcoin/bitcoin#12972
- bitcoin/bitcoin#13281
- bitcoin/bitcoin#13385
- bitcoin/bitcoin#13041
- bitcoin/bitcoin#13454
- bitcoin/bitcoin#13448
- bitcoin/bitcoin#13510
- bitcoin/bitcoin#13851
- bitcoin/bitcoin#13863
- bitcoin/bitcoin#14115
- bitcoin/bitcoin#14831
- bitcoin/bitcoin#15164
- bitcoin/bitcoin#15170
- bitcoin/bitcoin#15166
- bitcoin/bitcoin#16036
- bitcoin/bitcoin#16768

Several of the lints fail for our current codebase; these will be addressed in a
subsequent PR.
zkbot added a commit to zcash/zcash that referenced this pull request Nov 9, 2020
@zkbot
Auto merge of #4829 - str4d:backport-lints, r=str4d
84a5830 
Backport useful lints from upstream

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6892
- bitcoin/bitcoin#11151
- bitcoin/bitcoin#11300
- bitcoin/bitcoin@96d91b7
- bitcoin/bitcoin#12097
- bitcoin/bitcoin#12098
- bitcoin/bitcoin#12442
- bitcoin/bitcoin#12572
- bitcoin/bitcoin#12757
- bitcoin/bitcoin#11878
- bitcoin/bitcoin#12933
- bitcoin/bitcoin#12871
- bitcoin/bitcoin#12972
- bitcoin/bitcoin#13281
- bitcoin/bitcoin#13385
- bitcoin/bitcoin#13041
- bitcoin/bitcoin#13454
- bitcoin/bitcoin#13448
- bitcoin/bitcoin#13510
- bitcoin/bitcoin#13851
- bitcoin/bitcoin#13863
- bitcoin/bitcoin#14115
- bitcoin/bitcoin#14831
- bitcoin/bitcoin#15164
- bitcoin/bitcoin#15170
- bitcoin/bitcoin#15166
- bitcoin/bitcoin#16036
- bitcoin/bitcoin#16768
- bitcoin/bitcoin#13494

Several of the lints fail for our current codebase; these will be addressed in a
subsequent PR.
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
@laanwj @PastaPastaPasta
Merge bitcoin#15170: refactor/lint: Add ignored shellcheck suggestion…
a54f338 
…s to an array

cbd9091 refactor/lint: Add ignored suggestions to an array (Vidar Holen)

Pull request description:

  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.

Tree-SHA512: 04afced1d27fda940cc5e61d7f9ed04507c8f7f7dfd0031c09898a599c6de93695923a80cb3d515a0f0bf728847592d8680c15ac2e376b48726c03ca744f13a5
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Aug 24, 2021
@laanwj @UdjinM6
Merge bitcoin#15219: lint: Enable python linters via an array
34614f8 
948d8f4 lint: Enable python linters via an array (Ben Woosley)

Pull request description:

  This assures consistent recording of the enabled linters.

  This applies the same fix as bitcoin#15170 to lint-python.sh

Tree-SHA512: 6d03f919e86e7c2465475c88b25dd84391282bcc11728078024daf0432a7dccddf9e4a2cdae35d6ef374971cb4e12f0fa21b58f757e25f2fe7c12ceb4f4b2c57
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
@laanwj @Munkybooty
Merge bitcoin#15219: lint: Enable python linters via an array
846473a 
948d8f4 lint: Enable python linters via an array (Ben Woosley)

Pull request description:

  This assures consistent recording of the enabled linters.

  This applies the same fix as bitcoin#15170 to lint-python.sh

Tree-SHA512: 6d03f919e86e7c2465475c88b25dd84391282bcc11728078024daf0432a7dccddf9e4a2cdae35d6ef374971cb4e12f0fa21b58f757e25f2fe7c12ceb4f4b2c57
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@Empact Empact Empact left review comments

@promag promag promag left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@koalaman @Empact @DrahtBot @practicalswift @laanwj @promag @fanquake