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

chore(docs): inline-documentation #397

Conversation

theGaryLarson
Copy link
Contributor

@theGaryLarson theGaryLarson commented Jul 26, 2023

Checklist:

Starting on #391

I am keeping this as a draft PR until we can get the paths updated between tests and the API routes. Then will resubmit once tests clear.

I am using the AI tool Mintlify to help me understand the codebase and document it along the way. I believe this would benefit many people like myself who are new to the codebase and want to gain a better understanding.

@theGaryLarson
Copy link
Contributor Author

@lloydchang I added in-line docs for the api folder to start. Created this as a draft to get some feedback first though.

@lloydchang
Copy link
Contributor

lloydchang commented Jul 27, 2023

Thanks, @theGaryLarson

See #366 (comment)
and #396 (comment)

as they relate to:

Some checks were not successful
1 failing and 2 successful checks

Classroom ci / Build and test (16.14.2) (pull_request) Failing after 43s
Details

below.

@lloydchang
Copy link
Contributor

lgtm

@lloydchang
Copy link
Contributor

:shipit: 🚢 🇮🇹

@lloydchang
Copy link
Contributor

lloydchang commented Jul 27, 2023

While Mintlify Writer AI generated documentation is lengthy, I agree with @theGaryLarson that:

This would benefit many people like myself who are new to the codebase and want to gain a better understanding.


• In a professional setting, experienced software engineers would find the documentation too verbose.

• In this classroom setting, the target audience of this codebase is students who are learning the basics of software engineering and computer programming.


To use a basic example:

res.status(200).end();

is documented by Mintlify Writer as:

/* The line res.status(200).end(); is setting the HTTP response status code to 200 (OK) and ending the response. This
indicates that the request was successful and there is no additional data to be sent in the response body. */
res.status(200).end();


• Some students have not learned that 200 is OK.

• Other students and experienced software engineers learned from experience that 200 is OK.

• Most students and experienced software engineers have not read the official HTTP standard at https://datatracker.ietf.org/doc/html/rfc9110

• The Mintlify Writer-generated documentation tries to split the difference by adding lengthy documentation, which would be too verbose for experienced software engineers but is just the right temperature for students who are new to the codebase and want to understand better.


For reference, HTTP standard:

200 OK from the Hypertext Transfer Protocol (HTTP) standard by Internet Engineering Task Force (IETF) at https://datatracker.ietf.org/doc/html/rfc9110#section-15.3

Screen Shot 2023-07-26 at 8 01 11 PM

@lloydchang lloydchang mentioned this pull request Jul 27, 2023
2 tasks
@theGaryLarson theGaryLarson marked this pull request as ready for review July 27, 2023 21:56
@theGaryLarson theGaryLarson requested a review from a team as a code owner July 27, 2023 21:56
@theGaryLarson theGaryLarson marked this pull request as draft July 27, 2023 21:57
@theGaryLarson
Copy link
Contributor Author

Hey @lloydchang given the state of tests and route changes in API. I am going to leave this as a draft until tests pass. Then will sync and repush to this branch once they are resolved.

Slack convo with @ngillux

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.

2 participants