Set up CI with Azure Pipelines that checks that sources are clang-format'd #132
Conversation
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
BillyONeal
force-pushed
the
azure-pipelines
branch
from
904136e
to
88d8517
Compare
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
BillyONeal
changed the title
|
@Microsoft/vclibs Can you folks review this now? |
|
@StephanTLavavej Can you take a look at the readme.md changes? |
azure-pipelines.yml
Outdated
| - task: BatchScript@1 | ||
| inputs: | ||
| filename: 'ado/enforce-clang-format.cmd' | ||
| failOnStandardError: true |
There was a problem hiding this comment.
Can you add a newline to the end of this file?
ado/enforce-clang-format.cmd
Outdated
| @@ -0,0 +1,4 @@ | |||
| @echo off | |||
There was a problem hiding this comment.
Please add our license banner as a comment.
ado/enforce-clang-format.cmd
Outdated
| @@ -0,0 +1,4 @@ | |||
| @echo off | |||
| clang-format -style=file -i stl/inc/** stl/src/** 2>&1 | |||
There was a problem hiding this comment.
These ** double stars are interesting; who is parsing them? According to my understanding, this is not cmd syntax.
There was a problem hiding this comment.
Same: Billy replied to my comment that clang-format parses them.
There was a problem hiding this comment.
...judging from later changes, that must not have been the case. @BillyONeal?
ado/enforce-clang-format.cmd
Outdated
| @@ -0,0 +1,4 @@ | |||
| @echo off | |||
| clang-format -style=file -i stl/inc/** stl/src/** 2>&1 | |||
There was a problem hiding this comment.
How do we know what version of clang-format is being used? Where does it come from?
There was a problem hiding this comment.
It comes from "that's how Billy set up the hardware this task runs on". I'm going to put the list of what I did on the wiki.
There was a problem hiding this comment.
ado/enforce-clang-format.cmd
Outdated
| @@ -0,0 +1,4 @@ | |||
| @echo off | |||
There was a problem hiding this comment.
I suspect that we shouldn't abbreviate Azure DevOps to ado, even in the directory tree.
README.md
Outdated
| @@ -31,7 +31,9 @@ soon as possible.) | |||
| * Tests: **Coming soon.** We rely on three test suites: devcrt, tr1, and [libcxx][]. We need to replace our current test | |||
| harness, which extensively uses Microsoft-internal machinery. | |||
|
|
|||
| * Continuous Integration: **Coming soon.** We need tests first. | |||
| * Continuous Integration: **In progress.** We have set up Azure Pipelines to validate changes of the repository. | |||
| However, that infrastructure still requires manual review before building community submitted pull requests, as we | |||
There was a problem hiding this comment.
Should probably hyphenate community-submitted as a compound adjective.
There was a problem hiding this comment.
You and Casey think alike.
README.md
Outdated
| @@ -31,7 +31,9 @@ soon as possible.) | |||
| * Tests: **Coming soon.** We rely on three test suites: devcrt, tr1, and [libcxx][]. We need to replace our current test | |||
| harness, which extensively uses Microsoft-internal machinery. | |||
|
|
|||
| * Continuous Integration: **Coming soon.** We need tests first. | |||
| * Continuous Integration: **In progress.** We have set up Azure Pipelines to validate changes to the repository. | |||
There was a problem hiding this comment.
Nitpick: "We've" would be more consistent with the tone elsewhere. Similarly, "haven't yet hardened it".
| :: Copyright (c) Microsoft Corporation. | ||
| :: SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| clang-format -style=file -i stl/inc/* stl/inc/cvt/* stl/inc/experimental/* stl/src/* 2>&1 | ||
| echo If your build fails here, you need to format the following files with clang-format 8.0.1 |
azure-pipelines.yml
Outdated
| @@ -0,0 +1,12 @@ | |||
| # | |||
There was a problem hiding this comment.
I think the leading and trailing # should be dropped, and instead separated with a newline - unless there is some auto-formatter fighting against this.
azure-pipelines.yml
Outdated
| inputs: | ||
| filename: 'azure-devops/enforce-clang-format.cmd' | ||
| failOnStandardError: true | ||
| displayName: 'Verify clang-format' |
There was a problem hiding this comment.
Enforce, or verify? Terminology should be consistent. ALL SHALL LOVE CLANG-FORMAT AND DESPAIR.
This script runs clang-format on the sources, then runs
git status --porcelainon the result, redirected to stderr. If any output goes to stderr, Azure Pipelines marks the build as failed, which enforces that no changes were made to the repo by the clang-format pass.Resolves #88