Closed metalicjames closed 2 years ago
I really like a lot of the simplifications present in this. @jallen-frb can some/all of these simplifications be folded into (e.g., by rebasing your work on top of these commits) #161?
Looks good to me. Will be testing locally shortly.
tested ACK. This looks good and behaves well for me. My only question is about whether or not some of the proposed changes from #171 should be included here to minimize breakage potential.
@HalosGhost I think those changes probably make most sense to be included in #170. Since the snappy linkage issue doesn't seem to occur on Ubuntu or MacOS (which are the current supported platforms).
@HalosGhost I think those changes probably make most sense to be included in #170. Since the snappy linkage issue doesn't seem to occur on Ubuntu or MacOS (which are the current supported platforms).
My build env is Arch Linux and snappy library package doesn't include static in opposite to ubuntu packages.
This PR:
Dockerfile
, usingconfigure.sh
instead.clang-tidy
. Disables the newreadability-identifier-length
check for now, as large parts of the codebase are not compliant (perhaps this should be an issue?). This is needed to supportNOLINTBEGIN
andNOLINTEND
, which will be used in a subsequent PR.configure.sh
to remove the dependency cache, which I don't think is used in our current iteration of Github actions, or when building using docker.lint.sh
regex to only include files with the*pp
extension, rather than any file ending inpp
.This probably conflicts with #161.