Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AStyle addition to travis #6590

Merged
merged 8 commits into from
Apr 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .astyleignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
BUILD
cmsis
features/mbedtls
features/FEATURE_LWIP/lwip
rtos/TARGET_CORTEX/rtx4
features/filesystem/littlefs/littlefs
features/filesystem/fat/ChaN
features/frameworks
features/FEATURE_BLE/targets
features/FEATURE_LWIP/lwip-interface/lwip
features/unsupported/
features/FEATURE_COMMON_PAL/
FEATURE_NANOSTACK/coap-service
FEATURE_NANOSTACK/sal-stack-nanostack
rtos/TARGET_CORTEX/rtx5
targets
tools
34 changes: 34 additions & 0 deletions .astylerc
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Mbed OS code style definition file for astyle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asking for clarification. Some of these flags and comments sound like they modify the files themselves. Is this actually the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modify the files? can you elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do flags such as convert-tabs and indent-cases modify files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they do as any other flags there. astyle runs, changes files according to this style definition.


# Don't create backup files, let git handle it
suffix=none

# K&R style
style=kr

# 1 TBS addition to k&r, add braces to one liners
# Use -j as it was changed in astyle from brackets to braces, this way it is compatible with older astyle versions
-j

# 4 spaces, convert tabs to spaces
indent=spaces=4
convert-tabs

# Indent switches and cases
indent-switches
indent-cases

# Remove spaces in and around parentheses
unpad-paren

# Insert a space after if, while, for, and around operators
pad-header
pad-oper

# Pointer/reference operators go next to the name (on the right)
align-pointer=name
align-reference=name

# Attach { for classes and namespaces
attach-namespaces
attach-classes
39 changes: 39 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,45 @@ matrix:
# Report success since we have overridden default behaviour
- bash -c "$STATUS" success "Local $NAME testing has passed"

- env:
- NAME=astyle
install:
- wget https://downloads.sourceforge.net/project/astyle/astyle/astyle%203.1/astyle_3.1_linux.tar.gz;
mkdir -p BUILD && tar xf astyle_3.1_linux.tar.gz -C BUILD;
pushd BUILD/astyle/build/gcc;
make;
export PATH=$PWD/bin:$PATH;
popd;
- astyle --version
script:
# only changed files this time
- git diff --name-only $TRAVIS_BRANCH | grep '.*\.\(h\|c\|hpp\|cpp\)' | fgrep -v -f .astyleignore | xargs -n 100 -I {} bash -c "astyle -n --options=.astylerc \"{}\"" > astyle-files-changed.out;
if [ $(cat astyle-files-changed.out | grep Formatted | wc -l) -ne 0 ]; then
git --no-pager diff;
echo "Please fix style issues as shown above";
else
echo "Coding style check OK";
fi
after_success:
# run astyle for all files on the branch
- git checkout -- .
- find -regex '.*\.\(h\|c\|hpp\|cpp\)' -type f | fgrep -v -f .astyleignore | xargs -n 100 -I {} bash -c "astyle -n --options=.astylerc \"{}\"" > astyle-branch.out;
# update status if we succeeded, compare with master if possible
- |
CURR=$(cat astyle-branch.out | grep Formatted | wc -l)
PREV=$(curl https://api.github.com/repos/$TRAVIS_REPO_SLUG/status/master \
| jq -re "select(.sha != \"$TRAVIS_COMMIT\")
| .statuses[] | select(.context == \"travis-ci/$NAME\").description
| capture(\", (?<warnings>[0-9]+) warnings\").warnings" \
|| echo 0)

STATUSM="Passed, ${CURR} warnings"
if [ "$PREV" -ne 0 ]
then
STATUSM="$STATUSM ($(python -c "print '%+d' % ($CURR-$PREV)") warnings)"
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is, once master has an astyle status, PRs will say "Passed, 4 warnings (+4 warnings)". We should probably remove this script from here if we don't want to compare with master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we not want to compare with master? I would think that having warning delta would be useful, and part of this PR's intent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment for what's needed to compare with master. As is this is currently broken. (The "+4 warnings" is not a comparison)

- bash -c "$STATUS" success "$STATUSM"

- env:
- NAME=events
- EVENTS=events
Expand Down