Skip to content
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.

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

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

Merged
merged 1 commit into from Jan 16, 2019
Merged

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

merged 1 commit into from Jan 16, 2019

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.

This avoids duplicating the codes between command and comments.
@fanquake fanquake added the Tests label Jan 15, 2019
@Empact
Copy link
Member

@Empact Empact commented Jan 15, 2019

Concept ACK

@DrahtBot
Copy link
Contributor

@DrahtBot 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.

Copy link
Member

@promag promag left a comment

utACK.

BTW, have you verified remaining linters?

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
Member

@Empact Empact Jan 15, 2019

nit: duplicate

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
Member

@Empact Empact Jan 15, 2019

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

@Empact
Copy link
Member

@Empact Empact commented Jan 15, 2019

utACK cbd9091 - confirmed the same checks are disabled

@practicalswift
Copy link
Contributor

@practicalswift 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 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
2 checks passed
laanwj added a commit that referenced this issue Jan 16, 2019
… 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
laanwj added a commit that referenced this issue Jan 21, 2019
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
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jun 26, 2021
…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
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jun 28, 2021
…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
Munkybooty added a commit to Munkybooty/dash that referenced this issue Aug 21, 2021
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 added a commit to Munkybooty/dash that referenced this issue Aug 23, 2021
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 added a commit to Munkybooty/dash that referenced this issue Aug 24, 2021
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 added a commit to Munkybooty/dash that referenced this issue Aug 24, 2021
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 added a commit to Munkybooty/dash that referenced this issue Aug 24, 2021
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
UdjinM6 added a commit to UdjinM6/dash that referenced this issue Aug 24, 2021
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 added a commit to Munkybooty/dash that referenced this issue Aug 24, 2021
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants