Skip to content

Enable clippy in CI #174

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
Merged
merged 2 commits into from
Oct 29, 2019
Merged

Enable clippy in CI #174

merged 2 commits into from
Oct 29, 2019

Conversation

m-ou-se
Copy link
Contributor

@m-ou-se m-ou-se commented Oct 29, 2019

No description provided.

@rust-highfive
Copy link

r? @thejpster

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Oct 29, 2019
@m-ou-se m-ou-se force-pushed the clippy branch 2 times, most recently from da2a5e9 to cbd9389 Compare October 29, 2019 07:55
@korken89
Copy link
Contributor

I think this issue ties a back to (and extends) our original discussion on enabling code style checks as well (can't find the issue right now...).
It's probably time to start that discussion again and take a decision. Comments @rust-embedded/all ?
Or should we first start with clippy and then expand to fmt checks?

@m-ou-se
Copy link
Contributor Author

m-ou-se commented Oct 29, 2019

The main reason I'd like Clippy in CI here, is so #![deny(clippy::missing_inline_in_public_items)] can be added, to prevent people from accidentally adding trivial functions without #[inline]. (E.g. #173)

@m-ou-se m-ou-se force-pushed the clippy branch 2 times, most recently from a3d2bd5 to fed3855 Compare October 29, 2019 08:29
@m-ou-se m-ou-se marked this pull request as ready for review October 29, 2019 08:48
@m-ou-se m-ou-se requested a review from a team as a code owner October 29, 2019 08:48
@thejpster
Copy link
Contributor

Happy to start with clippy, and add cargo fmt -- --check separately, as that may require a reformat of the codebase.

@thejpster
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Oct 29, 2019
@bors @m-ou-se
Merge #174
3753237 
174: Enable clippy in CI r=thejpster a=m-ou-se



Co-authored-by: Mara Bos <[email protected]>
@bors
Copy link
Contributor

bors bot commented Oct 29, 2019

Build failed

@thejpster
Copy link
Contributor

Er, how do we find out why bors is sad?

@m-ou-se
Copy link
Contributor Author

m-ou-se commented Oct 29, 2019 •
edited
Loading

Sometimes the curl command to download GCC from the ARM website fails. It was in the travis logs bors linked, before the build was restarted.

@therealprof
Copy link
Contributor

Build failed due to network error, I've retriggered a rebuild to make sure (unfortunately I hit the wrong button so it's not just retrying the one job that had failed).

@therealprof
Copy link
Contributor

bors retry

bors bot added a commit that referenced this pull request Oct 29, 2019
@bors @m-ou-se
Merge #174
130d2d8 
174: Enable clippy in CI r=thejpster a=m-ou-se



Co-authored-by: Mara Bos <[email protected]>
@thejpster
Copy link
Contributor

Sometimes the curl command to download GCC from the ARM website fails. It was at the travis logs bors linked, before the build was restarted.

Hmm. Clicked all the little links and tiny buttons. Failed to see the great big blue link in the comment bors posted. Thanks!

@bors
Copy link
Contributor

bors bot commented Oct 29, 2019

Build succeeded

@bors bors bot merged commit 2d4b6e7 into rust-embedded:master Oct 29, 2019
@m-ou-se m-ou-se mentioned this pull request Oct 29, 2019
Merged
@m-ou-se m-ou-se deleted the clippy branch October 30, 2019 07:54
bors bot added a commit that referenced this pull request Nov 28, 2019
@bors @m-ou-se
Merge #175
ce538b2 
175: Enable the missing_inline_in_public_items clippy lint. r=jonas-schievink a=m-ou-se

This adds `#![deny(clippy::missing_inline_in_public_items)]` to make sure all functions in this crate are marked `#[inline]`, unless they are explicitly marked with `#[allow(clippy::missing_inline_in_public_items)]`.

Only three functions in this crate are not `#[inline]`:

 - `write_words`
 - `write_all`
 - `write_aligned`

Additionally, the derived `Debug` impl's also have a non-inline implementations.
This unfortunately means that the allow attribute also needs to added to any types deriving `Debug`.

See also #171 and #174 (comment).

Co-authored-by: Mara Bos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thejpster thejpster thejpster approved these changes

@therealprof therealprof therealprof approved these changes

Assignees

@thejpster thejpster

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@m-ou-se @rust-highfive @korken89 @thejpster @therealprof