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

Replace trivial implementations of Default with #[derive(Default)] #1098

Merged
merged 1 commit into from Jul 14, 2018

Conversation

Projects
None yet
4 participants
@niklasad1
Copy link
Member

niklasad1 commented Jul 8, 2018

Pull Request Overview

This pull request removes needless handwritten default implementations in the capsules

Testing Strategy

This pull request was tested by...

TODO or Help Wanted

This pull request has been tested on:

  • ADC
  • IPC
  • AppFlash
  • UART
  • I2C (failed on nRF52, need to activate the driver)
  • SD-card
  • SPI

(affects all boards but this changes _should_ be safe because all impls in the standard library is ok, https://doc.rust-lang.org/src/core/default.rs.html#139-#158 but I haven't check all impls in Tock but I assume that all options has None as default value!)

Documentation Updated

- [ ] Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.
@bradjc

This comment has been minimized.

Copy link
Contributor

bradjc commented Jul 9, 2018

I tried this on hail with the ADC test app and it seemed fine.

@alevy

alevy approved these changes Jul 9, 2018

@alevy alevy changed the title Remove needless default implementations in Capsules Replace trivial implementations of Default with #[derive(Default)] Jul 9, 2018

@alevy alevy added the P-Upkeep label Jul 9, 2018

@niklasad1

This comment has been minimized.

Copy link
Member

niklasad1 commented Jul 9, 2018

I wonder when I finally manage to get a PR title unchanged 🦀 🦀

@alevy

This comment has been minimized.

Copy link
Member

alevy commented Jul 9, 2018

I wonder when I finally manage to get a PR title unchanged

It's not personal!! I pick on yours because your PRs are nice and feature-focused, so they're easiest to find good titles for. But almost all of our PRs have bad titles.

@ppannuto

This comment has been minimized.

Copy link
Member

ppannuto commented Jul 14, 2018

I think this has gotten the testing it will get at this point / needs to be confident that default values for Options are doing what we expect

@ppannuto ppannuto merged commit 21a0876 into master Jul 14, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@ppannuto ppannuto deleted the capsules/remove-needless-default-impl branch Jul 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment