Re: [PATCH] Improvements to "Getting started" tutorial for Google Code-in task

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, LAM JUN RONG <h1710074(at)nushigh(dot)edu(dot)sg>
Cc: Andreas 'ads' Scherbaum <ads(at)pgug(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Improvements to "Getting started" tutorial for Google Code-in task
Date: 2019-01-05 15:21:23
Message-ID: f5d48c80-15c7-476e-7ef6-aeff81b0ebf4@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 04/01/2019 00:05, Alvaro Herrera wrote:
> Besides that, I have a hard time considering this patch committable.
> There are some good additions, but they are mixed with some wording
> changes that seem to be there just because the author doesn't like the
> original, not because they're actual improvements.

I agree. I don't find any of the changes to be improvements.

> diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
> index 4487d0cfd1..460a2279b6 100644
> --- a/doc/src/sgml/installation.sgml
> +++ b/doc/src/sgml/installation.sgml
> @@ -75,7 +75,7 @@ su - postgres
> <application>make</application> programs or older <acronym>GNU</acronym> <application>make</application> versions will <emphasis>not</emphasis> work.
> (<acronym>GNU</acronym> <application>make</application> is sometimes installed under
> the name <filename>gmake</filename>.) To test for <acronym>GNU</acronym>
> - <application>make</application> enter:
> + <application>make</application> and check its version, enter:
> <screen>
> <userinput>make --version</userinput>
> </screen>

This might be worthwhile, but it kind of restates something obvious.

> @@ -385,8 +385,8 @@ su - postgres
> This script will run a number of tests to determine values for various
> system dependent variables and detect any quirks of your
> operating system, and finally will create several files in the
> - build tree to record what it found. You can also run
> - <filename>configure</filename> in a directory outside the source
> + build tree to record what it found. If it does not print any error messages, configuration was successful.
> + You can also run <filename>configure</filename> in a directory outside the source
> tree, if you want to keep the build directory separate. This
> procedure is also called a
> <indexterm><primary>VPATH</primary></indexterm><firstterm>VPATH</firstterm>

This just says that if there were no errors, it was successful. I think
we don't need to state that. The (from-source) installation chapter is
not for beginners.

> @@ -1610,6 +1610,17 @@ su - postgres
> <screen>
> All of PostgreSQL successfully made. Ready to install.
> </screen>
> + If you see an error message like:
> +<screen>
> +ERROR: `flex' is missing on your system. It is needed to create the
> +file `bootscanner.c'. You can either get flex from a GNU mirror site
> +or download an official distribution of PostgreSQL, which contains
> +pre-packaged flex output.
> +</screen>
> + then packages which are required to build
> + <productname>PostgreSQL</productname> are missing.
> + See <xref linkend="install-requirements"/> for a list of requirements.
> +
> </para>
>
> <para>

This says, if there was an error, you need to read what it says and follow
the instructions. Again, see above.

Moreover, I don't want to keep copies of particular error messages in the
documentation that we then need to keep updating.

> diff --git a/doc/src/sgml/start.sgml b/doc/src/sgml/start.sgml
> index 5b73557835..e29672a505 100644
> --- a/doc/src/sgml/start.sgml
> +++ b/doc/src/sgml/start.sgml
> @@ -19,9 +19,8 @@
>
> <para>
> If you are not sure whether <productname>PostgreSQL</productname>
> - is already available or whether you can use it for your
> - experimentation then you can install it yourself. Doing so is not
> - hard and it can be a good exercise.
> + is already available for your experimentation,
> + you can install it yourself, which is not complicated.
> <productname>PostgreSQL</productname> can be installed by any
> unprivileged user; no superuser (<systemitem>root</systemitem>)
> access is required.

Doesn't look like a necessary change to me.

> @@ -83,7 +82,7 @@
>
> <listitem>
> <para>
> - The user's client (frontend) application that wants to perform
> + The user's client (frontend), an application that wants to perform
> database operations. Client applications can be very diverse
> in nature: a client could be a text-oriented tool, a graphical
> application, a web server that accesses the database to

Doesn't look like an improvement to me.

> @@ -153,7 +152,7 @@
> <screen>
> <prompt>$</prompt> <userinput>createdb mydb</userinput>
> </screen>
> - If this produces no response then this step was successful and you can skip over the
> + If this exits without any error message then this step was successful and you can skip over the
> remainder of this section.
> </para>
>

This removes the valuable information that in the success case, no output
is printed.

> @@ -240,12 +239,14 @@ createdb: database creation failed: ERROR: permission denied to create database
> <para>
> You can also create databases with other names.
> <productname>PostgreSQL</productname> allows you to create any
> - number of databases at a given site. Database names must have an
> - alphabetic first character and are limited to 63 bytes in
> - length. A convenient choice is to create a database with the same
> - name as your current user name. Many tools assume that database
> - name as the default, so it can save you some typing. To create
> - that database, simply type:
> + number of databases at a given site. However, if you would like to create databases with names that do not start with an alphabetic character,
> + you will need to quote the identifier, like "1234". The length of database names are limited to 63 bytes,
> + which is the default length defined in NAMEDATALEN. Changing this value requires recompiling the database.
> + Names longer than the set value will be truncated.
> + A convenient choice is to create a database with the same name as your current user name.
> + Many tools assume that database name as the default, so it
> + can save you some typing. To create that database, simply type:
> <screen>
> <prompt>$</prompt> <userinput>createdb</userinput>
> </screen>

The stuff about the quoting is wrong (shell != SQL). The stuff about
recompiling is unnecessary for a getting-started tutorial.

> @@ -355,7 +356,7 @@ mydb=#
> <para>
> The last line printed out by <command>psql</command> is the
> prompt, and it indicates that <command>psql</command> is listening
> - to you and that you can type <acronym>SQL</acronym> queries into a
> + to you and that you can type commands and <acronym>SQL</acronym> queries into a
> work space maintained by <command>psql</command>. Try out these
> commands:
> <indexterm><primary>version</primary></indexterm>

What's the difference between a "command" and an "SQL query" one might
ask here. Doesn't look like useful change.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-01-05 15:42:47 Re: Use atexit() in initdb and pg_basebackup
Previous Message Peter Eisentraut 2019-01-05 15:02:31 Re: [proposal] Add an option for returning SQLSTATE in psql error message