Skip to content

Commit 40cc52d

Browse files
robertodrarnfinn
authored andcommitted
Add Danger.Systems integration to the project (#73)
This commit set ups integration of the project with Danger.Systems, in order to automatize some common reviewing tasks. Danger.Systems is a Ruby gem that is installed and run in one the Travis CI jobs and checks PRs for compliance with a set of rules defined in the Dangerfile. A bot is configured to report the results of the following checks for any PR: 1. Commit linting 2. Additions to documentation and tests 3. Additions to CHANGELOG.md 4. Compliance to the code style Tasks accomplished in this commit: - The .travis.yml file was updated to run the Danger gem - The .clang-format file was regenerated based on clang-format v3.9.1 - The contributing guidelines in CONTRIBUTING.md now mention the comments that will be posted to the PR by the bot - Documentation about maintenance was updated to describe the role of Danger.Systems and the usage of Git pre-commit hooks - The shebang in the shell scripts used as Git pre-commit hooks was fixed - The license_maintainer.py script was adjusted in order to be Python 3 compatible. - Danger.Systems uses v0.1.0 of code style validation plugin. The plugin comes from the official, upstream repository. Thanks @nikolaykasyanov!!!
1 parent b2f42b2 commit 40cc52d

File tree

8 files changed

+361
-25
lines changed

8 files changed

+361
-25
lines changed

.clang-format

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,67 @@
1+
Language: Cpp
12
AccessModifierOffset: -2
2-
AlignAfterOpenBracket: true
3+
AlignAfterOpenBracket: Align
4+
AlignConsecutiveAssignments: false
5+
AlignConsecutiveDeclarations: false
36
AlignEscapedNewlinesLeft: false
4-
AlignOperands: true
7+
AlignOperands: true
58
AlignTrailingComments: true
69
AllowAllParametersOfDeclarationOnNextLine: false
710
AllowShortBlocksOnASingleLine: false
811
AllowShortCaseLabelsOnASingleLine: false
912
AllowShortFunctionsOnASingleLine: All
1013
AllowShortIfStatementsOnASingleLine: false
1114
AllowShortLoopsOnASingleLine: false
12-
AlwaysBreakAfterDefinitionReturnType: false
15+
AlwaysBreakAfterDefinitionReturnType: None
16+
AlwaysBreakAfterReturnType: None
1317
AlwaysBreakBeforeMultilineStrings: false
1418
AlwaysBreakTemplateDeclarations: false
1519
BinPackArguments: false
1620
BinPackParameters: false
21+
BraceWrapping:
22+
AfterClass: false
23+
AfterControlStatement: false
24+
AfterEnum: false
25+
AfterFunction: false
26+
AfterNamespace: false
27+
AfterObjCDeclaration: false
28+
AfterStruct: false
29+
AfterUnion: false
30+
BeforeCatch: false
31+
BeforeElse: false
32+
IndentBraces: false
1733
BreakBeforeBinaryOperators: None
1834
BreakBeforeBraces: Attach
1935
BreakBeforeTernaryOperators: true
2036
BreakConstructorInitializersBeforeComma: false
21-
ColumnLimit: 85
22-
CommentPragmas: '^ IWYU pragma:'
37+
BreakAfterJavaFieldAnnotations: false
38+
BreakStringLiterals: true
39+
ColumnLimit: 85
40+
CommentPragmas: '^ IWYU pragma:'
2341
ConstructorInitializerAllOnOneLineOrOnePerLine: true
2442
ConstructorInitializerIndentWidth: 4
2543
ContinuationIndentWidth: 4
2644
Cpp11BracedListStyle: true
2745
DerivePointerAlignment: false
28-
DisableFormat: false
46+
DisableFormat: false
2947
ExperimentalAutoDetectBinPacking: false
48+
ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ]
49+
IncludeCategories:
50+
- Regex: '^"(llvm|llvm-c|clang|clang-c)/'
51+
Priority: 2
52+
- Regex: '^(<|"(gtest|isl|json)/)'
53+
Priority: 3
54+
- Regex: '.*'
55+
Priority: 1
56+
IncludeIsMainRegex: '$'
3057
IndentCaseLabels: true
31-
IndentWidth: 2
58+
IndentWidth: 2
3259
IndentWrappedFunctionNames: false
60+
JavaScriptQuotes: Leave
61+
JavaScriptWrapImports: true
3362
KeepEmptyLinesAtTheStartOfBlocks: true
34-
Language: Cpp
63+
MacroBlockBegin: ''
64+
MacroBlockEnd: ''
3565
MaxEmptyLinesToKeep: 1
3666
NamespaceIndentation: None
3767
ObjCBlockIndentWidth: 2
@@ -44,16 +74,18 @@ PenaltyBreakString: 1000
4474
PenaltyExcessCharacter: 1000000
4575
PenaltyReturnTypeOnItsOwnLine: 200
4676
PointerAlignment: Middle
77+
ReflowComments: true
78+
SortIncludes: true
4779
SpaceAfterCStyleCast: false
4880
SpaceBeforeAssignmentOperators: true
4981
SpaceBeforeParens: ControlStatements
5082
SpaceInEmptyParentheses: false
5183
SpacesBeforeTrailingComments: 1
52-
SpacesInAngles: false
53-
SpacesInCStyleCastParentheses: false
84+
SpacesInAngles: false
5485
SpacesInContainerLiterals: true
86+
SpacesInCStyleCastParentheses: false
5587
SpacesInParentheses: false
5688
SpacesInSquareBrackets: false
57-
Standard: Cpp03
58-
TabWidth: 8
59-
UseTab: Never
89+
Standard: Cpp03
90+
TabWidth: 8
91+
UseTab: Never

.travis.yml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
language: cpp
22
sudo: false
33
dist: trusty
4+
ruby: 2.2.0
45
notifications:
56
slack:
67
secure: F2nQIW6SiaGdw1LjuZOlgBu8rUVMllrDG/5bhmTQP7gyETfViFBjTsHQdTle6jtdb+LudleZaG7WhdEiVcKUa834rKqDk1UOt9p6bsmgbsBZBAaxmPh01iVFhKn3ML7JLjfr1YtH7MWJcsS60cNBrohXfVKfFzNgDqZEu/llr90=
78
cache:
89
ccache: true
10+
bundler: true
911
directories:
1012
- "$HOME/.ccache"
1113
env:
@@ -36,13 +38,15 @@ matrix:
3638
- Fortran_COMPILER='gfortran-4.6'
3739
- BUILD_TYPE='release'
3840
- PYTHON_VER='2.7'
41+
- RUN_DANGER=false
3942
- os: linux
4043
addons:
4144
apt:
4245
packages:
4346
- cmake3
4447
- cmake3-data
4548
- clang
49+
- clang-format-3.9
4650
- gfortran-4.6
4751
- libboost-math-dev
4852
env:
@@ -52,6 +56,7 @@ matrix:
5256
- BUILD_TYPE='release'
5357
- PYTHON_VER='3.5'
5458
- STATIC='--static'
59+
- RUN_DANGER=true
5560

5661
- os: osx
5762
osx_image: xcode7.3
@@ -63,6 +68,7 @@ matrix:
6368
- PYTHON_VER='2.7'
6469
- HOMEBREW_GCC='homebrew/versions/gcc5'
6570
- STATIC='--static'
71+
- RUN_DANGER=false
6672
- os: osx
6773
osx_image: xcode7.3
6874
env:
@@ -72,6 +78,7 @@ matrix:
7278
- BUILD_TYPE='release'
7379
- PYTHON_VER='3.5'
7480
- HOMEBREW_GCC='homebrew/versions/gcc6'
81+
- RUN_DANGER=false
7582

7683
- os: linux
7784
addons: *1
@@ -82,6 +89,7 @@ matrix:
8289
- BUILD_TYPE='debug'
8390
- PYTHON_VER='2.7'
8491
- COVERAGE='--coverage'
92+
- RUN_DANGER=false
8593
allow_failures:
8694
- os: linux
8795
addons: *1
@@ -92,6 +100,7 @@ matrix:
92100
- BUILD_TYPE='debug'
93101
- PYTHON_VER='2.7'
94102
- COVERAGE='--coverage'
103+
- RUN_DANGER=false
95104
install:
96105
- |
97106
if [[ "${PYTHON_VER}" == "2.7" ]]; then
@@ -125,10 +134,18 @@ before_script:
125134
- ${CXX_COMPILER} --version
126135
- ${C_COMPILER} --version
127136
- ${Fortran_COMPILER} --version
137+
- |
138+
if [[ "${RUN_DANGER}" = true ]]; then
139+
bundle install
140+
fi
141+
script:
142+
- |
143+
if [[ "${RUN_DANGER}" = true ]]; then
144+
bundle exec danger
145+
fi
128146
- python setup.py --cxx=${CXX_COMPILER} --cc=${C_COMPILER} --fc=${Fortran_COMPILER} --type=${BUILD_TYPE} --cmake-options='-Hprojects/CMake' ${STATIC} ${COVERAGE}
129147
- cd build
130148
- ../.scripts/ci_build.sh
131-
script:
132149
- python ../.scripts/ci_test.py ctest --output-on-failure --verbose
133150
- python ../.scripts/ci_print_failing.py
134151
after_success:

CONTRIBUTING.md

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@ Our contribution guide is based on [Psi4 contribution guide](https://github.com/
2727
Note that after you launch a PR from one of your fork's branches, all
2828
subsequent commits to that branch will be added to the open pull request
2929
automatically.
30-
Each commit added to the PR will be validated for mergability, compilation
30+
Each commit added to the PR will be validated for mergeability, compilation
3131
and test suite compliance; the results of these tests will be visible on the
3232
PR page.
33+
* The title of your pull request should be marked with `[WIP]` if it’s a work
34+
in progress and with `#trivial` if it is a set of trivial changes.
3335
* If you're providing a new feature, you must add test cases, documentation and
3436
update the `CHANGELOG.md` file.
3537
* When the code is ready to go, make sure you run the full or relevant portion
@@ -40,6 +42,36 @@ Our contribution guide is based on [Psi4 contribution guide](https://github.com/
4042
integration (Travis for Linux and Mac) returns checkmarks, and multiple core
4143
developers give "Approved" reviews.
4244

45+
## Pull Request Requirements
46+
47+
The project is integrated with [Danger.Systems](http://danger.systems/ruby/).
48+
On each PR, one CI job will run the integration and a [bot](https://github.com/minazobot) will
49+
report which requirements are **not met** in your PR.
50+
These reports can be _warnings_ and _errors_. You will discuss and solve both
51+
of them with the reviewers.
52+
The automatic rules are laid out in the `Dangerfile` and are used to enforce an
53+
adequate level of testing, documentation and code quality.
54+
55+
### Danger.Systems Warnings
56+
57+
- PRs classed as Work in Progress.
58+
- Codebase was modified, but no tests were added.
59+
- Nontrivial changes to the codebase, but no documentation added.
60+
- Codebase was modified, but `CHANGELOG.md` was not updated.
61+
- Source files were added or removed, but `.gitattributes` was not updated.
62+
63+
### Danger.Systems Errors
64+
65+
- Commit message linting, based on some of [these recommendations](https://chris.beams.io/posts/git-commit/):
66+
- Commit subject is more than one word.
67+
- Commit subject is no longer than 50 characters.
68+
- Commit subject and body are separated by an empty line.
69+
70+
- Clean commit history, without merge commits.
71+
72+
- Code style for `.hpp`, `.cpp`, `.h` files follows the conventions in
73+
`.clang-format`.
74+
4375
## Licensing
4476

4577
We do not require any formal copyright assignment or contributor license

Dangerfile

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# Adapted from https://github.com/capistrano/danger/blob/master/Dangerfile
2+
# Q: What is a Dangerfile, anyway? A: See http://danger.systems/
3+
4+
# ------------------------------------------------------------------------------
5+
# Additional pull request data
6+
# ------------------------------------------------------------------------------
7+
pr_number = github.pr_json["number"]
8+
pr_url = github.pr_json["_links"]["html"]["href"]
9+
# Sometimes its a README fix, or something like that - which isn't relevant for
10+
# including in a CHANGELOG for example
11+
declared_trivial = (github.pr_title + github.pr_body).include?("#trivial")
12+
13+
# Just to let people know
14+
warn("PR is classed as Work in Progress") if github.pr_title.include? "[WIP]"
15+
16+
# Ensure a clean commits history
17+
if git.commits.any? { |c| c.message =~ /^Merge branch '#{github.branch_for_base}'/ }
18+
fail('Please rebase to get rid of the merge commits in this PR')
19+
end
20+
commit_lint.check disable: [:subject_cap, :subject_period]
21+
22+
# Check code style with clang-format
23+
code_style_validation.check file_extensions: ['.hpp', '.cpp', '.h'], ignore_file_patterns: [/^external\//]
24+
25+
# ------------------------------------------------------------------------------
26+
# What changed?
27+
# ------------------------------------------------------------------------------
28+
# Were source files added? Were source files deleted?
29+
# We look for added and removed files in the api, include, src, tests and tools directories.
30+
# Files whose license header might need to be updated are kept here.
31+
has_added_files = !git.added_files.grep(/^(api|include|src|tests|tools)/).empty?
32+
has_deleted_files = !git.deleted_files.grep(/^(api|include|src|tests|tools)/).empty?
33+
# Was any code modified in the api, cmake, external, include, src and tools directories?
34+
has_code_changes = !git.modified_files.grep(/^(api|cmake|external|include|src|tools)/).empty?
35+
# Was any code modified in the tests directory?
36+
has_test_changes = !git.modified_files.grep(/^tests/).empty?
37+
# Was the CHANGELOG.md file modified?
38+
has_changelog_changes = git.modified_files.include?("CHANGELOG.md")
39+
# Was the .gitattributes file modified?
40+
# .gitattributes is used to keep track of license headers and has to be updated
41+
# if files are added or removed
42+
has_gitattributes_changes = git.modified_files.include?(".gitattributes")
43+
# Was documentation added?
44+
has_doc_changes = !git.modified_files.grep(/^doc/).empty?
45+
46+
# ------------------------------------------------------------------------------
47+
# You've made changes to api|cmake|external|include|src|tools,
48+
# but didn't write any tests?
49+
# ------------------------------------------------------------------------------
50+
if has_code_changes && !has_test_changes
51+
if %w(tests).any? { |dir| Dir.exist?(dir) }
52+
warn("There are code changes, but no corresponding tests. "\
53+
"Please include tests if this PR introduces any modifications in "\
54+
"behavior.",
55+
:sticky => false)
56+
else
57+
markdown <<-MARKDOWN
58+
Thanks for the PR! This project lacks automated tests, which makes reviewing and approving PRs somewhat difficult. Please make sure that your contribution has not broken backwards compatibility or introduced any risky changes.
59+
60+
MARKDOWN
61+
end
62+
end
63+
64+
# ------------------------------------------------------------------------------
65+
# You've made nontrivial changes to api|cmake|external|include|src|tools,
66+
# but didn't write any docs?
67+
# ------------------------------------------------------------------------------
68+
doc_changes_recommended = git.insertions > 15
69+
if has_code_changes && !has_doc_changes && doc_changes_recommended && not_declared_trivial
70+
warn("Consider adding supporting documentation to this change. Documentation sources can be found in the `doc` directory.")
71+
end
72+
73+
# ------------------------------------------------------------------------------
74+
# Have you updated CHANGELOG.md?
75+
# ------------------------------------------------------------------------------
76+
if !has_changelog_changes && has_code_changes
77+
markdown <<-MARKDOWN
78+
Here's an example of a CHANGELOG.md entry:
79+
80+
```markdown
81+
* [##{pr_number}](#{pr_url}): #{github.pr_title} - [@#{github.pr_author}](https://github.com/#{github.pr_author})
82+
```
83+
MARKDOWN
84+
warn("Please update CHANGELOG.md with a description of your changes. "\
85+
"If this PR is not a user-facing change (e.g. just refactoring), "\
86+
"you can disregard this.", :sticky => false)
87+
end
88+
89+
# ------------------------------------------------------------------------------
90+
# You've made changes to api|include|src|tests|tools,
91+
# but didn't update .gitattributes?
92+
# ------------------------------------------------------------------------------
93+
if has_added_files && has_deleted_files && !has_gitattributes_changes
94+
warn("You have added source files without updating `.gitattributes`.", :sticky => false)
95+
end
96+
if has_deleted_files && !has_gitattributes_changes
97+
warn("You have removed source files without updating `.gitattributes`.", :sticky => false)
98+
end
99+
100+
lgtm.check_lgtm

Gemfile

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# frozen_string_literal: true
2+
source "https://rubygems.org"
3+
4+
gem "danger"
5+
gem "danger-commit_lint"
6+
gem "danger-lgtm"
7+
gem 'danger-code_style_validation', '0.1.0' , :git => 'https://github.com/flix-tech/danger-code_style_validation.git'

0 commit comments

Comments
 (0)