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

Provide non blocking API #16

Merged
merged 3 commits into from
May 26, 2020
Merged

Provide non blocking API #16

merged 3 commits into from
May 26, 2020

Conversation

rnestler
Copy link
Collaborator

@rnestler rnestler commented Mar 3, 2020

No description provided.

@rnestler rnestler requested a review from dbrgn March 3, 2020 20:47
@codecov
Copy link

codecov bot commented Mar 3, 2020

Codecov Report

Merging #16 into master will increase coverage by 1.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
+ Coverage   85.34%   86.47%   +1.12%     
==========================================
  Files           3        3              
  Lines         116      133      +17     
==========================================
+ Hits           99      115      +16     
- Misses         17       18       +1     
Impacted Files Coverage Δ
src/lib.rs 83.33% <100.00%> (+2.01%) ⬆️

That way the user can choose to just start a measurement and read the
results later.
@rnestler rnestler changed the title WIP: Provide non blocking API Provide non blocking API May 12, 2020
src/lib.rs Outdated Show resolved Hide resolved
Copy link
Owner

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Can you provide multiple impl blocks (common functionality, blocking API, non-blocking API) so we get nicer docs?

Most of the time delays need to be used all over in the system, thus we
can't just take ownership of it.
@rnestler rnestler requested a review from dbrgn May 12, 2020 18:52
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Copy link
Owner

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Thanks! There are some more things that should be done before the next release:

  • Provide an async wakeup function as well
  • Add notes about async to README and to crate-level documentation
  • Add an example that uses async calls (ideally an embedded one)

However all of them can be added in separate PRs, so let's merge this.

@rnestler can you squash the last two commits before merging?

That way the documentation is easier to read, since the async and
blocking measurement functions are separate.

Co-authored-by: Danilo Bargen <[email protected]>
@rnestler rnestler merged commit 5913a16 into master May 26, 2020
@rnestler rnestler deleted the provide-non-blocking-api branch May 26, 2020 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants