From: | Pierre Ducroquet <p(dot)psql(at)pinaraf(dot)info> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Cc: | Jan Michálek <godzilalalala(at)gmail(dot)com> |
Subject: | Re: Other formats in pset like markdown, rst, mediawiki |
Date: | 2017-03-23 16:26:03 |
Message-ID: | 20170323162603.22419.16605.pgcf@coridan.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Mat Arye | 2017-03-23 16:33:46 | Order-preserving function transforms and EquivalenceClass |
Previous Message | Charles Cui | 2017-03-23 16:13:35 | Fwd: GSOC 2017 project ideas |