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

WIP: Add more documentation to trustfall_core #136

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ginger51011
Copy link
Contributor

This is a very WIP project related to #135.

Right now there isn't much documentation at all, so I thought I might help get the rustdoc at least a bit more easy to navigate. It's a bit write-as-I-go, but I'm open to feedback and suggestions from anyone!

@obi1kenobi
Copy link
Owner

This is awesome, thank you so much for taking the initiative on this 🎉

I'm currently exhausted from staying up half the night answering questions on HackerNews, but I will read and review ASAP.

For easier merging, consider splitting up the docs so that the additions to each file are its own pull request? That way each file can be documented, discussed, and merged separately from all the other files. In my experience, this will make us able to move a lot faster.

@ginger51011
Copy link
Contributor Author

This is awesome, thank you so much for taking the initiative on this 🎉

I'm currently exhausted from staying up half the night answering questions on HackerNews, but I will read and review ASAP.

For easier merging, consider splitting up the docs so that the additions to each file are its own pull request? That way each file can be documented, discussed, and merged separately from all the other files. In my experience, this will make us able to move a lot faster.

Thanks! No stress at all, it's pretty crazy seeing the repo going from ~500 stars to 1,1k in like a day!

This could probably get merged now (if the quality is up to par), and I could organize further PRs in a more organized way. This is pretty much me trying to understand the code by writing notes here and there

@obi1kenobi
Copy link
Owner

Thanks! No stress at all, it's pretty crazy seeing the repo going from ~500 stars to 1,1k in like a day!

Apparently Trustfall was the #​1 most-starred project on GitHub yesterday: https://twitter.com/PredragGruevski/status/1623711641127747584

Absolutely wild! 🚀

This could probably get merged now (if the quality is up to par), and I could organize further PRs in a more organized way. This is pretty much me trying to understand the code by writing notes here and there

Based on a quick glance I think some small edits for clarity would be in order, largely due to my poor choices of names that then require more documentation than otherwise would have been required.

Let's split the difference: would you mind splitting this into PR-per-directory, and using PR-per-file for the rest?

You probably know this already, but just in case: git checkout <branch> <path> is super helpful for splitting up PRs like this. It's like a cross-branch copy-paste for the specified files: check out a new branch from main, then git checkout docs/improve-docs <path> to copy an entire file or directory over to the new branch, which you can then use to open a PR. So it should only be a few minutes' worth of work to split up this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for the query language or library APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants