Conversation
…ar as a true backend
…s even if already reported
samwillis
left a comment
samwillis
left a comment
There was a problem hiding this comment.
Looking very good. So clean compared to what we had before.
Mostly what I would like to see is some comments and docs emailing whats happening. May be a README-PGLITE-DEV.md in the root with an overview of what we have changed and why, with links to relevant place.
👏
There was a problem hiding this comment.
Could we extensively comment this file?
Postgres does a great job of adding a large doc comment at the top of each source file explaining what it does, then commenting each function or blocks of globals indicating what they are for and how they work. Can we do the same?
There is so much embedded knowledge in here, it would be good to surface it (will make it 10x easer to review too!)
| static void disable_statement_timeout(void); | ||
|
|
||
|
|
||
| /* these must be volatile to ensure state is preserved across longjmp: */ |
There was a problem hiding this comment.
Once longjmp is documented / commented on in pglitec.c can you add a comment here saying "see pglitec.c for details"
There was a problem hiding this comment.
Git is doing a bad job of showing the diff. I think this is mostly the unrolled main loop. Ensure there is a comment explaining what we have done.
2 participants
PR for inspecting changes introduced for PGlite.