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

Re-enable lint checks #1079

Merged
merged 13 commits into from
May 8, 2019
Merged

Re-enable lint checks #1079

merged 13 commits into from
May 8, 2019

Conversation

cmihai
Copy link
Member

@cmihai cmihai commented May 7, 2019

This PR tries to fix and re-enable the lint checks that were disabled for the 0.17 merge. Enabled checks:

  • Git subtree checks for src/crypto/ctaes, src/secp256k1, src/univalue and src/leveldb
  • lint-circular-dependencies.sh, lint-filenames.sh, lint-format-strings.sh, lint-includes.sh, lint-locale-dependence.sh, lint-logs.sh, lint-python-shebang.sh, lint-python-utf8-encoding.sh, lint-shell-locale.sh, lint-whitespace.sh

The last several checks require extensive code changes, so I'd like to submit them in separate PRs.

Part of #1054.

@cmihai cmihai added the wip Work in progress which is not supposed to be merged yet label May 7, 2019
@cmihai cmihai self-assigned this May 7, 2019
@scravy scravy self-requested a review May 7, 2019 13:20
@scravy
Copy link
Member

scravy commented May 7, 2019

Concept ACK

Copy link
Member

@cornelius cornelius left a comment

Choose a reason for hiding this comment

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

Yay for reenabling the checks. ConceptACK.

.travis/lint_06_script.sh Outdated Show resolved Hide resolved
test/lint/lint-all.sh Show resolved Hide resolved
@cmihai cmihai added the tests Automated tests label May 7, 2019
cmihai added 9 commits May 7, 2019 17:40
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Copy link
Member

@Gnappuraz Gnappuraz left a comment

Choose a reason for hiding this comment

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

ConceptACK ae52550

src/esperanza/finalizationstate.cpp Show resolved Hide resolved
test/lint/lint-all.sh Outdated Show resolved Hide resolved
test/lint/lint-circular-dependencies.sh Show resolved Hide resolved
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
@cmihai cmihai removed the wip Work in progress which is not supposed to be merged yet label May 8, 2019
Copy link
Member

@scravy scravy left a comment

Choose a reason for hiding this comment

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

utACK 9b279c1

Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
@scravy
Copy link
Member

scravy commented May 8, 2019

Screenshot 2019-05-08 12 54 32

A linting step failed, probably due to rebase with #1080

Had this pull request been merged before mine I'd have had to justfiy introducing a boost dependency, so let me speak here ;-)

This is a dependency which is only introduced in tests + we do not have any intention to move away from the boost unit tests. While in general the idea is to migrate boost-features to equivalent C++ native features (like favor std::option over boost::optional, not yet available in C++11 though), we are not discouraging reusability and proper software engineering. The dependency is not exactly new as in it already is available to us and it's actually required to use a piece of functionality which boost unit test already provides (and we are using the boost unit test framework after all).

So I guess it should be added to the known boost dependencies.

cmihai added 2 commits May 8, 2019 14:36
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>
Copy link
Member

@cornelius cornelius left a comment

Choose a reason for hiding this comment

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

utACK 128f7a9

@cmihai cmihai merged commit b3cc3e0 into dtr-org:master May 8, 2019
@cmihai cmihai deleted the fix-lint branch May 8, 2019 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Automated tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants