The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passed
Hi
This is my first review (Magnus said in his presentation in PGDay Paris that volunteers should just come and help, so here I am), so please notify me for any mistake I do when using the review tools...
The feature seems to work as expected, but I don't claim to be a markdown and rst expert. Some minor issues with the code itself : - some indentation issues (documentation and code itself with mix between space based and tab based indentation) and a few trailing spaces in code - typographic issues in the documentation : - "The html, asciidoc, latex, latex-longtable, troff-ms, and markdown and rst formats" ==> duplicated and - "Sets the output format to one of unaligned, aligned, wrapped, html, asciidoc, latex (uses tabular), latex-longtable, rst, markdown, or troff-ms." ==> extra comma at the end of the list - the comment " dont add line after last row, because line is added after every row" is misleading, it should warn that it's only for rst - there is a block of commented out code left - in the print_aligned_vertical function, there is a mix between "cont->opt->format == PRINT_RST" and "format == &pg_rst" and I don't see any obvious reason for that - the documentation doesn't mention (but ok, it's kind of obvious) that the linestyle option will not work with rst and markdown
Thanks !
The new status of this patch is: Waiting on Author
Corrected problem with \pset linestyle when format is set to markdown, or rst.
Corrected tuples only for markdown and rst (normal and expanded)
