Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

fix(docs): better docs for the settings module #121

Merged
merged 2 commits into from Jul 6, 2018
Merged

Conversation

philbooth
Copy link
Contributor

@philbooth philbooth commented Jul 6, 2018

A couple of small documentation fixes, not associated with any issue:

  • Add a docstring to the derive_and_validate macro expansion. All the other types in settings have a doc comment and the generated docs looked incomplete without it for these too.

  • Fix up the docs for SqsUrls, introducing a newline to keep the outer summary short and moving another note up from the wrong place where it was associated with SqsUrls::bounce.

With these changes, the settings docs look like this:

Screen shot of the settings documentation

@brizental, @mozilla/fxa-devs r?

@philbooth philbooth changed the title Pb/settings docs fix(docs): better docs for the settings module Jul 6, 2018
@philbooth philbooth requested review from brizental and a team July 6, 2018 10:25
@philbooth philbooth self-assigned this Jul 6, 2018
Copy link

@shane-tomlinson shane-tomlinson left a comment

Choose a reason for hiding this comment

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

Like the last one, looks great to me, but someone with more rust experience should r+.

Copy link
Member

@brizental brizental left a comment

Choose a reason for hiding this comment

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

This looks great, @philbooth . Thanks.
I will remember to document everything that I add to the codebase properly from now on :)

@@ -22,7 +22,8 @@ use validate;
mod test;

macro_rules! deserialize_and_validate {
($(($type:ident, $validator:ident, $expected:expr)),+) => ($(
($(#[$docs:meta] ($type:ident, $validator:ident, $expected:expr)),+) => ($(
#[$docs]
Copy link
Member

Choose a reason for hiding this comment

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

Cool stuff, didn't know you had to do this in order to add comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't know you had to do this in order to add comments

No worries, it took me a while to figure out how to do it! 😄

@philbooth philbooth merged commit ba9f8b8 into master Jul 6, 2018
@philbooth philbooth deleted the pb/settings-docs branch July 6, 2018 16:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants