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

DHT module docs #51

Merged
merged 10 commits into from
Jan 7, 2025
Merged

DHT module docs #51

merged 10 commits into from
Jan 7, 2025

Conversation

ThetaSinner
Copy link
Member

@ThetaSinner ThetaSinner commented Dec 11, 2024

This PR now does three things. Some minor code cleanup and documentation consistency for existing code. Then I've also added some top level crate docs with diagrams and started a glossary. Initially this PR just had the glossary.


It might be too early for this, just thinking about where to put the glossary of terms that @zippy asked for. I think it's a good idea, we've already got a bunch of terms we use that wouldn't be clear to anybody who didn't work on Kitsune1 :)

I'm thinking the API is a decent place. Everything will have a trail back to the API, it's the starting point for anybody integrating with Kitsune2.
A better place might be the main module (the equivalent of kitsune_p2p) because that's where we'll have the maximum number of dependencies available to use for type references.

Doing a glossary like this, we get nice easy to reference links to terms -> file:////kitsune2/target/doc/kitsune2_api/doc/glossary/index.html#time-slice

@ThetaSinner ThetaSinner requested a review from a team December 11, 2024 21:47
jost-s
jost-s previously approved these changes Dec 11, 2024
Copy link
Contributor

@jost-s jost-s left a comment

Choose a reason for hiding this comment

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

Yeah, that's good. I think api crate and main crate are equally valid places for this. Slight preference for main crate.

crates/api/src/doc/glossary.rs Outdated Show resolved Hide resolved
crates/api/src/doc/glossary.rs Outdated Show resolved Hide resolved
crates/api/src/doc/glossary.rs Show resolved Hide resolved
neonphog
neonphog previously approved these changes Dec 11, 2024
@ThetaSinner ThetaSinner dismissed stale reviews from neonphog and jost-s via 5aa3901 December 12, 2024 11:14
@ThetaSinner ThetaSinner marked this pull request as ready for review December 12, 2024 11:22
@ThetaSinner
Copy link
Member Author

I think I prefer the main crate too. I will commit to moving this module there once it exists and for now, I'll add to this? :)

@jost-s
Copy link
Contributor

jost-s commented Dec 12, 2024

sure!

@ThetaSinner ThetaSinner force-pushed the add-doc-module branch 2 times, most recently from bd1021a to 478a77e Compare December 16, 2024 15:54
@ThetaSinner
Copy link
Member Author

I'll continue this after #59

@ThetaSinner ThetaSinner changed the title Add doc module DHT module docs Jan 6, 2025
crates/dht/build.rs Outdated Show resolved Hide resolved
@neonphog
Copy link
Collaborator

neonphog commented Jan 6, 2025

Suggestion: #66

crates/dht/build.rs Outdated Show resolved Hide resolved
neonphog
neonphog previously approved these changes Jan 6, 2025
Copy link
Collaborator

@neonphog neonphog left a comment

Choose a reason for hiding this comment

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

Some different options for the build.rs and svgs. All the text and changes look good to me!

Copy link
Collaborator

@neonphog neonphog left a comment

Choose a reason for hiding this comment

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

🎉

@ThetaSinner ThetaSinner enabled auto-merge (squash) January 7, 2025 15:52
@ThetaSinner ThetaSinner merged commit 9cd2ec6 into main Jan 7, 2025
5 checks passed
@ThetaSinner ThetaSinner deleted the add-doc-module branch January 7, 2025 16:25
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