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

Document the public API #65

Merged
merged 1 commit into from
Jun 4, 2016
Merged

Document the public API #65

merged 1 commit into from
Jun 4, 2016

Conversation

jwilm
Copy link
Contributor

@jwilm jwilm commented May 2, 2016

This adds documentation to the public APIs. I tried to highlight any backend specific gotchas in a couple of cases (such as which errors can be generated from where).

There are a few things I found that shouldn't really be part of the public API, but those can't be resolved until a new major version; I'll file another issue to address that.


This change is Reviewable

@jwilm
Copy link
Contributor Author

jwilm commented May 2, 2016

One other thing - I used the contents of the README for the main crate documentation. The README contents are a good starting point for this crate, and now they are now available when a user views docs locally.

It might be nice to setup hosted documentation with automatic updates via travis. Then, the README can be truncated and point to the hosted docs.

@jwilm
Copy link
Contributor Author

jwilm commented May 10, 2016

Any feedback on this? Sorry for the bump, but it's been over a week :).

Thanks!

@passcod
Copy link
Member

passcod commented May 11, 2016

Oh no! I'm terribly sorry, I'd misplaced this. I'll have a look asap. I really appreciate the work and discussion you've been doing :) (I've been in bed with the flu the last few days, which hasn't helped any.)

@passcod
Copy link
Member

passcod commented May 11, 2016

Can we enable a compile flag to warn on missing docs?

@jwilm
Copy link
Contributor Author

jwilm commented May 11, 2016

Thanks for looking at this, @passcod!

#![deny(missing_docs)] can be applied at the crate root, but I believe that would also require all the internal types to be documented as well (might be wrong on this). Will test that later..

@blaenk
Copy link
Contributor

blaenk commented May 11, 2016

If that's true, it wouldn't be possible to except certain things by doing something like #[allow(missing_docs)] on them?

@@ -53,10 +63,14 @@ struct StreamContextInfo {

impl FsEventWatcher {
#[inline]
#[doc(hidden)]
// TODO should not be pub
Copy link
Member

Choose a reason for hiding this comment

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

If these should not be pub, can we unpub them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opted not to unpub them since it could theoretically be a breaking change.

`#![deny(missing_docs)]` is now specified at the crate root which
requires public APIs to be documentated.
@jwilm
Copy link
Contributor Author

jwilm commented May 11, 2016

#![deny(missing_docs)] turned out to only affect the public API. The module doc formatting was homogenized to have no trailing comment line and to be followed by a blank line:

//! foo
//! blah blah blah

.. code ..

@passcod
Copy link
Member

passcod commented May 12, 2016

Can you document (at least sparsely) the windows variants? Otherwise we break the build

@passcod
Copy link
Member

passcod commented Jun 4, 2016

Finally getting to merge this after very busy weeks at work! I'll try to set up hosted docs somewhere...

@passcod passcod merged commit 0e6378c into notify-rs:master Jun 4, 2016
passcod added a commit that referenced this pull request Jun 4, 2016
@jwilm
Copy link
Contributor Author

jwilm commented Jun 4, 2016

Thanks for merging this! Somehow I missed your comment from 23 days ago...

Can you document (at least sparsely) the windows variants? Otherwise we break the build

Any action required there?

@jwilm jwilm deleted the docs branch June 4, 2016 16:32
@passcod
Copy link
Member

passcod commented Jun 4, 2016

I set missing_docs as warn-only for that file. If you can do it, please :) but otherwise it'll probably get done the next time the windows code is touched.

@jwilm
Copy link
Contributor Author

jwilm commented Jun 4, 2016

If it's not blocking anything, then I'm just going to abstain for now. Actively in the middle of another project :). Thanks again for merging!

@passcod passcod added this to the 2.6.0 milestone Jun 5, 2016
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.

3 participants