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

Group module docs #176

Merged
merged 1 commit into from
Dec 22, 2021
Merged

Group module docs #176

merged 1 commit into from
Dec 22, 2021

Conversation

patmaddox
Copy link
Contributor

I checked out the docs and it took quite a while to find what I was looking for. Maybe it's just unfamiliarity with Elixir conventions (e.g. plural-named modules have functions, singular-named modules are data types). I grouped the modules in a way that I think would make them more immediate to someone wanting to use this library. I picked "API", "Data Types", and "Util" as the group names - I'm certainly open to different groupings or names, but this made sense to me.

Before

image

After

image

@patmaddox patmaddox requested review from a team and OleMchls and removed request for a team December 15, 2021 02:24
@OleMchls OleMchls requested review from ecomba and removed request for OleMchls December 20, 2021 21:51
@OleMchls
Copy link
Contributor

@patmaddox thanks so much for providing this <3 I like these additions a lot, I assigned @ecomba to review the categories you picked, and hopefully, we can include this with the upcoming test release of #175

Copy link
Contributor

@ecomba ecomba left a comment

Choose a reason for hiding this comment

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

Hey @patmaddox !

I really like how you've grouped the code, it makes it easier to navigate the docs; thank you so much!

What do you think of adding another section (for completeness sake) for the dnsimple specific errors? Not sure if this would be to much though (I'll let you decide).

One thing I would like you to do though is add what you did in the CHANGELOG.md as per our CONTRIBUTING.md.

@patmaddox
Copy link
Contributor Author

Hey @patmaddox !

I really like how you've grouped the code, it makes it easier to navigate the docs; thank you so much!

What do you think of adding another section (for completeness sake) for the dnsimple specific errors? Not sure if this would be to much though (I'll let you decide).

Mix docs already break errors out into their own Exceptions section at the bottom. Here's the bottom of the module list (original screenshot only shows the top). Unless you mean something else that I overlooked?

image

@patmaddox
Copy link
Contributor Author

One thing I would like you to do though is add what you did in the CHANGELOG.md as per our CONTRIBUTING.md.

Done! :)

@ecomba
Copy link
Contributor

ecomba commented Dec 22, 2021

Hey @patmaddox !
I really like how you've grouped the code, it makes it easier to navigate the docs; thank you so much!
What do you think of adding another section (for completeness sake) for the dnsimple specific errors? Not sure if this would be to much though (I'll let you decide).

Mix docs already break errors out into their own Exceptions section at the bottom. Here's the bottom of the module list (original screenshot only shows the top). Unless you mean something else that I overlooked?

image

Duh... 😄

@ecomba
Copy link
Contributor

ecomba commented Dec 22, 2021

One thing I would like you to do though is add what you did in the CHANGELOG.md as per our CONTRIBUTING.md.

Done! :)

Just a little update. Can you put the comment over the 3.1.0 section? Basically right under main.

When we prepare a release we set a new version, which will contain the changes in the changelog for that version.

Otherwise, everything looks great!

Thank you so much for this! 🙏🏼

@patmaddox
Copy link
Contributor Author

Just a little update. Can you put the comment over the 3.1.0 section? Basically right under main.

Done!

@ecomba ecomba merged commit c6ae086 into dnsimple:main Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants