| 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: | Whole Thread | Raw Message | 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 |