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.

Sign up for GitHub

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

Jump to bottom

Fix astyle errors (clears all remaining styling issues) #8711

Merged
merged 24 commits into from
Nov 16, 2018
Merged

Fix astyle errors (clears all remaining styling issues) #8711

merged 24 commits into from
Nov 16, 2018

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Nov 12, 2018

Description

Last clean up for master - run astyle for all files.

Main goal: turn astyle coding failures to errors (I'll test it here that styling error causes PR red)

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@0xc0170 0xc0170 force-pushed the fix_astyle_error branch 3 times, most recently from 304eb57 to 427ad7a Compare November 12, 2018 12:33
@0xc0170 0xc0170 changed the title Fix astyle error Fix astyle errors (clears all remaining styling issues) Nov 12, 2018
@0xc0170 0xc0170 force-pushed the fix_astyle_error branch 3 times, most recently from 69bf4a8 to fc05eec Compare November 13, 2018 10:27
@cmonr cmonr self-requested a review November 13, 2018 22:51
@0xc0170
Copy link
Contributor Author

0xc0170 commented Nov 14, 2018

The style check works - it errors if there is style issue. I'll rebase to remove style issue and should get green.

This still needs reviews

@0xc0170
CThunkbase: fix coding style
43615ba
@0xc0170
blockdevice test: fix coding style
a4500e8
@0xc0170
blockdevice: fix components coding style
7d7bfd7
@0xc0170
rtc time test: fix coding style
501bc29
@0xc0170
astyleignore: add nanostack targets folder
9a0651c 
Targets will be styled separately
@0xc0170
rtx idle: fix coding style
830f746
@0xc0170
drivers: fix coding style for doxygen
fbdbfff
@0xc0170
platform: fix coding style
3d85957
@0xc0170
travis: astyle error leads to the pull request error
edef49b 
As our codebase is 0 styling errors, we should start treating astyle errors as
pull request blockers. Time to follow our coding style.
@0xc0170
filehandle test: fix coding style
53c4277
@0xc0170
tests: fix doxygen style
ef728d0
@0xc0170
rtos: fix coding style
e1e20a6
@0xc0170
nanostack mesh: fix coding style
3aab381
@0xc0170
nanostakc cmsis rtos: fix coding style
29a1149
@0xc0170
nanostack interface: fix coding style
eec4e13
@0xc0170
nanostack eventloop: fix coding style
3f9d1a9
@0xc0170
travis: add after_failure for astyle
70b5772 
To get to failure, we need to exit with 1
@0xc0170
events: fix coding style
d3e51fc
@0xc0170
Copy link
Contributor Author

0xc0170 commented Nov 15, 2018

Conflict resolved, removed style error for testing, rebased to the latest master

@0xc0170
Copy link
Contributor Author

0xc0170 commented Nov 15, 2018

8 styling issues on master, rerunning travis to report them and will fix

@0xc0170
Copy link
Contributor Author

0xc0170 commented Nov 15, 2018

AStyle - 0 errors, please review! This was tested with an error, and without.

I would suggest to get this for 5.11 (asap) otherwise we will have conflicts all the time (not that simple to rebase and rerun astyle to check for errors)

cmonr
cmonr approved these changes Nov 15, 2018
View reviewed changes
Copy link
Contributor

@cmonr cmonr left a comment

Choose a reason for hiding this comment

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

LGTM!

features/nanostack/mbed-mesh-api/source/ethernet_tasklet.c
enet_tasklet_network_state_changed(MESH_CONNECTED_GLOBAL);
} else {
enet_tasklet_network_state_changed(MESH_CONNECTED_LOCAL);;
}
}
} else {
if (tasklet_data_ptr->connection_status != MESH_DISCONNECTED &&
tasklet_data_ptr->connection_status != MESH_BOOTSTRAP_STARTED)
tasklet_data_ptr->connection_status != MESH_BOOTSTRAP_STARTED) {
enet_tasklet_network_state_changed(MESH_DISCONNECTED);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit concerned and bothered that astyle caught this..

rtos/TARGET_CORTEX/mbed_rtx_idle.cpp
if (NULL == os_timer) {
os_timer = new (os_timer_data) rtos::internal::SysTimer();
os_timer->setup_irq();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The way GitHub handled this diff is odd...

@cmonr
Copy link
Contributor

cmonr commented Nov 15, 2018

travis-ci/astyle — Passed, 0 files (-58 files)

Minus.

Nice

@cmonr cmonr mentioned this pull request Nov 16, 2018
Merged
@cmonr
Copy link
Contributor

cmonr commented Nov 16, 2018

Info: This PR has been re-bundled into a new rollup PR (#8763).

The previous rollup found an issue with a bundled PR after new devices were added into master. The PR needing work has been removed.

No further work is needed here, as once that PR is merged, this PR will also be closed and marked as merged.
If any more commits are made in this PR, this PR will remain open and have to go through CI on its own.

@0xc0170 0xc0170 merged commit 3f289c2 into ARMmbed:master Nov 16, 2018
@0xc0170 0xc0170 removed the needs: CI label Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmonr cmonr cmonr approved these changes

Assignees
No one assigned
Labels
priority: high release-version: 5.11.0-rc1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xc0170 @cmonr