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

chore: Ignore 3rd party headers in clang-tidy #51

Merged
merged 19 commits into from Jan 11, 2019

Conversation

Projects
None yet
3 participants
@ciband
Copy link
Collaborator

ciband commented Jan 5, 2019

Proposed changes

Added regex to ignore 3rd party headers and disabled cpp core guildeline checks until we bring in GSL.

Added NOLINT to GTest test macros to ignore GTest code.

Cleared clang tidy warnings in os/http.cpp

Types of changes

  • Refactoring (improve a current implementation without adding a new feature or fixing a bug)

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes

Further comments

N/A

ciband added some commits Jan 3, 2019

Merge pull request #1 from ArkEcosystem/master
merge master into master
chore: Limit clang-tidy to just cpp-crypto
Adjusted header-filter regex to filter out 3rd party libraries.
chore: Limit clang-tidy to just cpp-client
Adjusted header-filter regex to filter out 3rd party libraries.
chore: Skip GTest macro for clang tidy
Added // NOLINT to GTest macro to force clang tidy to skip it since we do not have control over the code.
chore: Limit clang-tidy to just cpp-crypto
Adjusted header-filter regex to filter out 3rd party libraries.
chore: Skip GTest macro for clang tidy
Added // NOLINT to GTest macro to force clang tidy to skip it since we do not have control over the code.
@sleepdefic1t
Copy link
Contributor

sleepdefic1t left a comment

Looks good 👍

This can be merged.

@ciband

This comment has been minimized.

Copy link
Collaborator

ciband commented Jan 10, 2019

Build failures are due to these tests:

/home/circleci/project/test/api/two/two_transactions.cpp:78: Failure
Expected equality of these values:
45024866
epoch
Which is: 45024880
[ FAILED ] api.test_two_transaction (55 ms)

/home/circleci/project/test/api/two/two_votes.cpp:77: Failure
Expected equality of these values:
45024867
epoch
Which is: 45024890
[ FAILED ] api.test_two_vote (54 ms)

I'm not sure why/how the epoch changed for the transaction that we are inspecting.

This issue will not be an issue once develop gets merged into main.

These build failures are not a result of this change.

@faustbrian

This comment has been minimized.

Copy link
Contributor

faustbrian commented Jan 11, 2019

@ciband conflicts

ciband added some commits Jan 11, 2019

chore: Add back clang tidy NOLINT
Added back clang-tidy NOLINT to ignore macros from GTest.

@ciband ciband dismissed stale reviews from faustbrian and sleepdefic1t via 2ce19a4 Jan 11, 2019

faustbrian and others added some commits Jan 11, 2019

@ciband

This comment has been minimized.

Copy link
Collaborator

ciband commented Jan 11, 2019

@faustbrian This PR should be good now.

@faustbrian faustbrian merged commit bfe5b3c into ArkEcosystem:master Jan 11, 2019

3 of 5 checks passed

ci/circleci: build-macos-9-2 Please select a macOS plan.
Details
ci/circleci: build-macos-9-3 Please select a macOS plan.
Details
ci/circleci: build-linux-clang-5 Your tests passed on CircleCI!
Details
ci/circleci: build-linux-default Your tests passed on CircleCI!
Details
ci/circleci: build-linux-gcc7 Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment