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
hereI 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
fewtrailing spaces in code
- typographic issues in the documentation : - "The html, asciidoc, latex, latex-longtable, troff-ms, and markdown and
rstformats" ==> duplicated and - "Sets the output format to one of unaligned, aligned, wrapped, html, asciidoc, latex
(usestabular), 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'sonly 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
