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

api->API #8461

Merged
merged 3 commits into from Oct 30, 2018

Conversation

Projects
None yet
5 participants
@kegilbert
Contributor

kegilbert commented Oct 17, 2018

Description

Minor doxygen update, uppercase acronym

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[X] Docs update
[ ] Test update
[ ] Breaking change
api->API
Minor doxygen update, uppercase acronym

@kegilbert kegilbert requested review from cmonr and AnotherButler Oct 17, 2018

Copy edit FileHandle.h
Copy edit file.
@AnotherButler

Please address my comments.

@@ -207,7 +207,7 @@ class FileHandle : private NonCopyable<FileHandle> {
/** Check for poll event flags
* The input parameter can be used or ignored - the could always return all events,

This comment has been minimized.

@AnotherButler

AnotherButler Oct 17, 2018

Contributor

Query: What should we replace "the" with in this sentence?

This comment has been minimized.

@kegilbert

kegilbert Oct 19, 2018

Contributor

@AnotherButler : Good catch, this is saying the caller can either pass in a filter for the events they want, or filter the return value and not use the argument depending on how this virtual function is implemented. I think that the was meant to be a they (meaning whoever is implementing this function).

This comment has been minimized.

@AnotherButler

AnotherButler Oct 19, 2018

Contributor

Could I rewrite this sentence as "You can use or ignore the input parameter. You can return all events or check just the events listed in events."?

This comment has been minimized.

@kegilbert

kegilbert Oct 19, 2018

Contributor

That sounds fine to me.

* Note! This is not intended as an attach-like asynchronous api, but rather
* as a building block for constructing such functionality.
* Note! This is not intended as an attach-like asynchronous API, but rather
* as a building block for constructing such functionality.
*
* The exact timing of when the registered function
* is called is not guaranteed and susceptible to change. It should be used

This comment has been minimized.

@AnotherButler

AnotherButler Oct 17, 2018

Contributor

Query: Is this saying that the registered function is not susceptible to change, or it is susceptible change?

This comment has been minimized.

@kegilbert

kegilbert Oct 19, 2018

Contributor

@geky

From my understanding this is saying this function alone is not sufficient for an async format but can be used to develop one.

This comment has been minimized.

@geky

geky Oct 23, 2018

Member

The timing of this function may change from release to release. Does that make sense?

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 19, 2018

@AnotherButler Is this waiting on you to resolve the queries, or @kegilbert to answer them?

Or @cmonr / @ARMmbed/mbed-os-maintainers / @ARMmbed/mbed-os-docs to provide some sort of answer?

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Oct 19, 2018

@cmonr @AnotherButler Sorry about that, my page wasn't loading properly when I submitted my comments before, they should be up now.

@0xc0170 0xc0170 closed this Oct 22, 2018

@0xc0170 0xc0170 deleted the kegilbert-patch-6 branch Oct 22, 2018

@AnotherButler

This comment has been minimized.

Contributor

AnotherButler commented Oct 22, 2018

Why did we close this?

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Oct 22, 2018

Ah, think it hit the 3 day mark due to the weekend. @0xc0170 I'm going to reopen this if that's alright.

@kegilbert kegilbert restored the kegilbert-patch-6 branch Oct 22, 2018

@kegilbert kegilbert reopened this Oct 22, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 22, 2018

It was closed because we were doing branch cleanup.

Someone opened a PR using ARMmbed as the user.

@0xc0170 0xc0170 removed the needs: review label Oct 22, 2018

@cmonr cmonr added the needs: work label Oct 23, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 23, 2018

A reminder that this still needs work.

Would be nice if this could be rolled up with the other docs PRs.

@cmonr cmonr added the rollup PR label Oct 23, 2018

@cmonr cmonr removed the rollup PR label Oct 24, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 29, 2018

@kegilbert

This comment has been minimized.

Contributor

kegilbert commented Oct 29, 2018

@cmonr This should be good to go.

@cmonr cmonr added needs: CI rollup PR and removed needs: work labels Oct 29, 2018

@0xc0170 0xc0170 merged commit 9ab13df into master Oct 30, 2018

12 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Passed, 545 files (+0 files)
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10044 cycles (-295 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8372B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@0xc0170 0xc0170 removed the needs: CI label Oct 30, 2018

@cmonr cmonr removed the rollup PR label Oct 30, 2018

@AnotherButler AnotherButler deleted the kegilbert-patch-6 branch Oct 30, 2018

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