Page MenuHomePhabricator

Enforce clang-format version 7.x
ClosedPublic

Authored by Fabien on Aug 6 2019, 08:54.

Details

Summary

llvm 8.0 and 7.1 have been released recently.

As an example, Ubuntu recently updated to clang-format 7.1, causing
arcanist to return an error on arc lint due to version conflict.

I tested our codebase against clang-format 7.1 and 8.0, there is
a minor difference in the output (some edge cases are causing
bad indents with clang-format-7, which is fixed by clang-format-8).

Therefore both versions cannot be allowed at the same time
as it would cause arc lint to format back and forth when
using different versions.

I would have preferred to enforce >= 7.0 and < 8 in the
.arclintfile but this is not supported by arcanist.
As a workaround an additional check is performed at the linter
level to enforce using version 7.x only.

Test Plan

With clang-format 7.0, 7.1 and 8.0:

arc lint --everything

Diff Detail

Repository
rABC Bitcoin ABC
Branch
fix_arclint_clang_format_version
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 7050
Build 12147: Bitcoin ABC Buildbot (legacy)
Build 12146: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Aug 6 2019, 16:13

Make it version 7.x or something. From a major version to another, clang format do change how it formats some things.

This revision now requires changes to proceed.Aug 6 2019, 16:13
Fabien planned changes to this revision.Aug 7 2019, 09:45
Fabien edited the test plan for this revision. (Show Details)
Fabien retitled this revision from Enforce clang-format version >= 7.0 to Enforce clang-format version 7.x.
Fabien edited the summary of this revision. (Show Details)

Perform an additional check at the linter level to enforce
7.x versions only.

This revision is now accepted and ready to land.Aug 8 2019, 05:37
This revision was automatically updated to reflect the committed changes.