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

Kernel: add comments #1121

Merged
merged 4 commits into from Aug 16, 2018

Conversation

Projects
None yet
6 participants
@bradjc
Copy link
Contributor

bradjc commented Jul 18, 2018

Pull Request Overview

This pull request:

  • Adds comments to data structures in the kernel.
  • Removes the need for Callback to be mut (which has ramifications for the capsules).
  • Removes the need for Allocator to be mut.

Blocked on #1115.

Testing Strategy

This pull request was tested by compiling.

TODO or Help Wanted

n/a

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.

bradjc added a commit that referenced this pull request Jul 18, 2018

@ppannuto ppannuto added the blocked label Jul 18, 2018

@phil-levis
Copy link
Collaborator

phil-levis left a comment

I think changes to the kernel that require every board, chip, and capsule to change require a design discussion before an implementation and pull request.

@bradjc

This comment has been minimized.

Copy link
Contributor

bradjc commented Jul 30, 2018

I think changes to the kernel that require every board, chip, and capsule to change require a design discussion before an implementation and pull request.

I tried. I got zero direct responses: #985 (comment)

@brghena

This comment has been minimized.

Copy link
Contributor

brghena commented Jul 30, 2018

@phil-levis It's pretty hard to have any take on this PR right now. It's blocked on changes that are actually in #1115, which in turn is blocked on changes that are actually in #1113. So a good first step would be to go to #1113 and take a look at that PR.

If you do take a look, you'll see that #1113 is based on a design discussion we had in #985. Once we handle both of those blocking commits, then we should revisit this PR and see how much it actually affects.

Take a look at #1113 first.

@bradjc bradjc force-pushed the kernel-improvements branch from 2d26b08 to 06df6ac Jul 31, 2018

bradjc added some commits Jul 17, 2018

@bradjc bradjc force-pushed the kernel-improvements branch from 06df6ac to e19b1d9 Aug 16, 2018

@bradjc bradjc changed the title Kernel: comments and `mut` removal Kernel: add comments Aug 16, 2018

@bradjc

This comment has been minimized.

Copy link
Contributor

bradjc commented Aug 16, 2018

Ok ready to review! This PR now just adds comments to the kernel crate.

@alevy

alevy approved these changes Aug 16, 2018

@alevy alevy added the P-Upkeep label Aug 16, 2018

@alevy

This comment has been minimized.

Copy link
Member

alevy commented Aug 16, 2018

bors r+

bors bot added a commit that referenced this pull request Aug 16, 2018

Merge #1121
1121: Kernel: add comments r=alevy a=bradjc

### Pull Request Overview

This pull request:

- Adds comments to data structures in the kernel.
- ~~Removes the need for `Callback` to be `mut` (which has ramifications for the capsules).~~
- ~~Removes the need for `Allocator` to be `mut`.~~

~~Blocked on #1115.~~

### Testing Strategy

This pull request was tested by compiling.


### TODO or Help Wanted

n/a


### Documentation Updated

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

### Formatting

- [x] Ran `make formatall`.


Co-authored-by: Brad Campbell <bradjc5@gmail.com>
@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Aug 16, 2018

@bors bors bot merged commit e19b1d9 into master Aug 16, 2018

4 checks passed

bors Build succeeded
Details
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

@bors bors bot deleted the kernel-improvements branch Aug 16, 2018

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