fsevents: improve documentation #52
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
houndci-bot
reviewed
Dec 17, 2021
42d0682 to
788cf07
Compare
houndci-bot
reviewed
Dec 17, 2021
This PR adds comprehensive doc comments to all exported constants, mostly derived from the Apple documentation but with changes to make them directly relevant to this package's API. It also uses C constants directly rather than relying on an implicit association between `1<<iota` and the actual enum values. It also unexports `GetStreamRefPaths`, `GetStreamRefDescription` and `GetStreamRefDeviceID` because none of them are usable with this package's API because there is no way to get an `FSEventStreamRef` value because it's hidden inside an unexported field inside the `EventStream` struct. Also unexport the `CFRunLoopRef` because it's similarly not useful or usable. Other than the above unexports, the API remains identical. Unfortunately I am unable to get the tests to pass, which makes me think that something is wrong somewhere, but that applies to the original code too, not just the code with these changes applied.
788cf07 to
bc450ee
Compare
nathany
reviewed
Jan 14, 2022
There was a problem hiding this comment.
This PR adds comprehensive doc comments to all exported
constants, mostly derived from the Apple documentation
That may be a violation of Apple's copyright? 🤔
TBH, this seems like it could be too much documentation. I'd rather keep the documentation light -- somewhere in between what it was and what you propose -- and then refer to Apple's docs for more depth.
| // only provide information to help you diagnose the problem. | ||
| KernelDropped = EventFlags(C.kFSEventStreamEventFlagKernelDropped) | ||
|
|
||
| // UserDropped is related to UserDropped above. |
There was a problem hiding this comment.
Suggested change
| // UserDropped is related to UserDropped above. | |
| // UserDropped is related to KernelDropped above. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds comprehensive doc comments to all exported
constants, mostly derived from the Apple documentation
but with changes to make them directly relevant to this package's API.
It also uses C constants directly rather than relying on an implicit
association between
1<<iotaand the actual enum values.It also unexports
GetStreamRefPaths,GetStreamRefDescriptionand
GetStreamRefDeviceIDbecause none of them are usablewith this package's API because there is no way to get an
FSEventStreamRefvalue because it's hidden inside an unexportedfield inside the
EventStreamstruct.Also unexport the
CFRunLoopRefbecause it's similarlynot useful or usable.
Other than the above unexports, the API remains identical.
Unfortunately I am unable to get the tests to pass, which
makes me think that something is wrong somewhere, but
that applies to the original code too, not just the code with
these changes applied.
What does this pull request do?
Where should the reviewer start?
How should this be manually tested?