Comments to understand functions in the backend #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Switch to non-native Postgres client. And add a "streaming" API for making database queries, which streams the results from the database to Node as they are generated by Postgres. This allows Node to process the rows one by one (and garbage collect in between), which is much easier on the VM when we need to do big queries that summarize data (or just format it and incrementally spit it out an HTTP response). * Mostly refactoring. This moves the handle_GET_reportExport route into its own file, which necessitated refactoring some other things (zinvite and pca) out of server.ts as well. Chipping away at the monolith. This also converts the votes.csv report to use the streaming query from Postgres, which is mostly a smoke test. It seems to work, so next I'll convert it to stream the results incrementally to the HTTP response as well. * Split each report into separate function. * Count up comment votes in single pass over votes table. There was actually a bug in the old SQL that aggregated votes from _all_ conversations instead of just the conversation in question, which is why it took 30 seconds to run. With that bug fixed, even the super slow "do a full subquery for each comment row" was actually quite fast. But this is way cheaper/faster. * Add participant-votes.csv export. * Switch to non-native Postgres client. And add a "streaming" API for making database queries, which streams the results from the database to Node as they are generated by Postgres. This allows Node to process the rows one by one (and garbage collect in between), which is much easier on the VM when we need to do big queries that summarize data (or just format it and incrementally spit it out an HTTP response). * Mostly refactoring. This moves the handle_GET_reportExport route into its own file, which necessitated refactoring some other things (zinvite and pca) out of server.ts as well. Chipping away at the monolith. This also converts the votes.csv report to use the streaming query from Postgres, which is mostly a smoke test. It seems to work, so next I'll convert it to stream the results incrementally to the HTTP response as well. * Split each report into separate function. * Count up comment votes in single pass over votes table. There was actually a bug in the old SQL that aggregated votes from _all_ conversations instead of just the conversation in question, which is why it took 30 seconds to run. With that bug fixed, even the super slow "do a full subquery for each comment row" was actually quite fast. But this is way cheaper/faster. * Add participant-votes.csv export. * Flip vote polarity. In the raw votes table, -1 means agree and 1 means disagree, so we need to count things correctly. And when exporting votes in participant votes, we flip the sign so that 1 means agree and -1 means disagree. * Properly escape comment text. * add votes matrix, show data license preprod, logging. --------- Co-authored-by: Michael Bayne <[email protected]>
This reverts commit 61d2940.
NewJerseyStyle
added
the
documentation
server/src/server.ts
Outdated
| @@ -234,6 +234,7 @@ function haltOnTimeout(req: { timedout: any }, res: any, next: () => void) { | |||
| } | |||
| } | |||
|
|
|||
| // checks if a property (name) exists in a source object and copies that property to a dest object | |||
There was a problem hiding this comment.
To enable IDEs recognize the comment and shows the comment when developer hover on the function where it is been used, the following syntax would be better:
/**
* Checks if a property exists in a source object and copies it to a destination object.
*
* @param name - The name of the property to check and copy.
* @param source - The source object containing the property.
* @param dest - The destination object to copy the property to. If undefined, a new object will be created.
* @returns The destination object with the property copied if it exists in the source object. Returns undefined if the source object is undefined.
* @throws {Error} If the source object is not an object or the destination object is not an object or undefined.
**/More about TSDoc specification. https://tsdoc.org/
server/src/server.ts
Outdated
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| "use strict"; | |||
|
|
|||
| import akismetLib from "akismet"; | |||
| import akismetLib from "akismet"; // spam protection for user-submitted text | |||
There was a problem hiding this comment.
Adding a comment block at the beginning of your file to explain the purpose and key features of imported libraries is a good practice, especially when those libraries are not immediately obvious from the code itself or are less commonly used.
Here's how you might structure such a comment block at the beginning of the file, using the akismet example:
/**
* @file This file contains the core logic for [brief description of the file's purpose].
*
* Dependencies:
* - `akismet`: This library is used for spam protection. It provides functionality to check if user-submitted text is spam. See [link to library documentation] for details on its API and usage. Specifically, we're using it to [explain how you're using it, e.g., "check comments before posting"].
* - [Other library]: [brief explanation of its purpose and how it's used in this file].
*
* Note: Ensure that the Akismet API key is correctly configured in the environment variables. See [link to relevant documentation or configuration file].
*/
import { Client as AkismetClient } from "akismet";
// ... rest of your code ...
|
It is trivial to manually edit all the comment for each function, you can used AI to assist you drafting the comment block and then review, refine it before commit. |
|
I'm going to close this for a sec, then re-open. that's just so I can try to sync the 
|
|
k that worked! I could click "sync fork" once I closed this. A bit wonky, but helpful to know :) Sorry for the noise in this PR |
|
I guess it is because Venish was using |
|
After implementing the TSDoc, it's all good to merge! @cplkake |
block comments generated by Gemini 1.5 Flash, detailed review is suggested (I roughly read through)
Comment generated by Gemini 1.5 Flash, more review is suggested
Comments generated by Gemini 1.5 Flash, more review suggested
NewJerseyStyle
approved these changes
Nov 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds comments describing what the following functions are set up to accomplish above each respective function:
server/src/server.ts
server/src/session.ts
server/src/user.ts