Skip to content

Commit 8f3e192

Browse files
authored
Source Code Format Checks (#380)
This change includes some tools to keep a consistent format of the repository's source files, including: - a `Checks` workflow which errors if any of a branch's changed files require formatting. We can extend this workflow with copyright header checks in the future and the pre-existing `Check PR Labels` has been folded into it. - updates to `install_prereqs_desktop.py` to ensure that clang-format is installed. - updates to `scripts/format_code.py` to reduce logging in non-verbose mode. - a pre-commit githook. Note that the githook has to be installed manually by copying it from `scripts/git/hooks/pre-commit` to `.git/hooks/pre-commit` - an update to our Contribution guidelines with formatting instructions.
1 parent a69445d commit 8f3e192

File tree

6 files changed

+83
-40
lines changed

6 files changed

+83
-40
lines changed

.github/workflows/check-labels.yml

Lines changed: 0 additions & 20 deletions
This file was deleted.

.github/workflows/checks.yml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
name: Checks
2+
3+
on:
4+
pull_request:
5+
types: [opened, reopened, synchronize, labeled, unlabeled]
6+
7+
env:
8+
triggerLabelFull: "tests-requested: full"
9+
triggerLabelQuick: "tests-requested: quick"
10+
statusLabelInProgress: "tests: in-progress"
11+
statusLabelFailed: "tests: failed"
12+
13+
jobs:
14+
check_integration_test_labels:
15+
# This check fails if integration tests are queued, in progress, or failed.
16+
runs-on: ubuntu-latest
17+
steps:
18+
- uses: docker://agilepathway/pull-request-label-checker:latest
19+
with:
20+
none_of: "${{ env.statusLabelInProgress }},${{ env.statusLabelFailed }},${{ env.triggerLabelFull }},${{ env.triggerLabelQuick }}"
21+
repo_token: ${{ github.token }}
22+
file_format_check:
23+
runs-on: ubuntu-latest
24+
steps:
25+
- uses: actions/checkout@v2
26+
with:
27+
submodules: false
28+
- name: Setup python
29+
uses: actions/setup-python@v2
30+
with:
31+
python-version: 3.7
32+
- name: Install prerequisites
33+
run: python scripts/gha/install_prereqs_desktop.py
34+
- name: log clang format version
35+
shell: bash
36+
run: clang-format --version
37+
- name: git fetch origin main
38+
shell: bash
39+
run: git fetch origin main
40+
- name: Detect Formatting Changes
41+
shell: bash
42+
run: python3 scripts/format_code.py -git_diff -noformat_file -verbose
43+
44+

CONTRIBUTING.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ Before you submit your pull request consider the following guidelines:
118118
* Follow our [Coding Rules](#rules).
119119
* Avoid checking in files that shouldn't be tracked (e.g `.tmp`, `.idea`).
120120
We recommend using a [global](#global-gitignore) gitignore for this.
121+
* Format your code:
122+
```shell
123+
python3 scripts/format_code.py -git_diff -verbose
124+
```
125+
121126
* Commit your changes using a descriptive commit message.
122127
123128
```shell

scripts/format_code.py

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,20 @@
3939
# Flag Definitions:
4040
FLAGS = flags.FLAGS
4141

42-
flags.DEFINE_boolean("git_diff", False, "Use git-diff to assemble a file list")
43-
flags.DEFINE_string("git_range", "origin/main..", "the range string when using "
44-
"git-diff.")
45-
flags.DEFINE_multi_string("f", None, "Append the filename to the list of "
46-
"files to check.")
47-
flags.DEFINE_multi_string("d", None,
48-
"Append the directory to the file list to format.")
49-
flags.DEFINE_boolean("r", False,
42+
flags.DEFINE_boolean('git_diff', False, 'Use git-diff to assemble a file list')
43+
flags.DEFINE_string('git_range', 'origin/main..', 'the range string when using '
44+
'git-diff.')
45+
flags.DEFINE_multi_string('f', None, 'Append the filename to the list of '
46+
'files to check.')
47+
flags.DEFINE_multi_string('d', None,
48+
'Append the directory to the file list to format.')
49+
flags.DEFINE_boolean('r', False,
5050
"Recurse through the directory's children When formatting a target directory.")
51-
flags.DEFINE_boolean("format_file", True, "Format files in place.")
52-
flags.DEFINE_boolean("verbose", False, "Execute in verbose mode.")
51+
flags.DEFINE_boolean('format_file', True, 'Format files in place.')
52+
flags.DEFINE_boolean("verbose", False, 'Execute in verbose mode.')
5353

5454
# Constants:
55-
FILE_TYPE_EXTENSIONS = (".cpp", ".cc", ".c", ".h")
55+
FILE_TYPE_EXTENSIONS = ('.cpp', '.cc', '.c', '.h')
5656
"""Tuple: The file types to run clang-format on.
5757
Used to filter out results when searching across directories or git diffs.
5858
"""
@@ -73,7 +73,7 @@ def does_file_need_formatting(filename):
7373
for line in result.stdout.decode('utf-8').splitlines():
7474
if line.strip().startswith("<replacement "):
7575
return True
76-
return False
76+
return False
7777

7878
def format_file(filename):
7979
"""Executes clang-format on the file to reformat it according to our
@@ -187,10 +187,14 @@ def is_file_objc_header(filename):
187187
bool: True if the header file contains known obj-c language definitions.
188188
Returns False otherwise.
189189
"""
190-
if '.h' not in filename:
191-
return False
192-
objective_c_tags = [ '@end', '@class', '#import']
193-
for line in open(filename):
190+
_, ext = os.path.splitext(filename)
191+
if ext != '.h':
192+
return False
193+
objective_c_tags = ('@end', '@class', '#import')
194+
195+
with open(filename) as header_file:
196+
lines = header_file.readlines()
197+
for line in lines:
194198
for tag in objective_c_tags:
195199
if tag in line:
196200
return True
@@ -211,8 +215,8 @@ def main(argv):
211215
filenames += git_diff_list_files()
212216

213217
exit_code = 0
214-
if 0 == len(filenames):
215-
print("No files to format.")
218+
if not filenames:
219+
print('No files to format.')
216220
sys.exit(exit_code)
217221

218222
if FLAGS.verbose:
@@ -224,7 +228,7 @@ def main(argv):
224228
if does_file_need_formatting(filename):
225229
if is_file_objc_header(filename):
226230
if FLAGS.verbose:
227-
print(' - Ignoring obj header: "{0}"'.format(filename))
231+
print(' - IGNORE OBJC: "{0}"'.format(filename))
228232
else:
229233
if FLAGS.verbose:
230234
print(' - FRMT: "{0}"'.format(filename))

scripts/gha/install_prereqs_desktop.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,18 @@ def main():
6464
if utils.is_linux_os():
6565
# sudo apt install ccache
6666
utils.run_command(['apt', 'install', '-y', 'ccache'], as_root=True)
67-
6867
elif utils.is_mac_os():
6968
# brew install ccache
7069
utils.run_command(['brew', 'install', 'ccache'])
70+
71+
# Install clang-format on linux/mac if its not installed already
72+
if not utils.is_command_installed('clang-format'):
73+
if utils.is_linux_os():
74+
# sudo apt install clang-format
75+
utils.run_command(['apt', 'install', '-y','clang-format'], as_root=True)
76+
elif utils.is_mac_os():
77+
# brew install protobuf
78+
utils.run_command(['brew', 'install', 'clang-format'])
7179

7280
# Install required python dependencies.
7381
# On Catalina, python2 in installed as default python.

scripts/git/hooks/pre-commit

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
echo .git/hooks/presubmit Checking File Formats
2+
python3 scripts/format_code.py -git_diff -noformat_file

0 commit comments

Comments
 (0)