In my quest to learn more about Drupal 7, I found several errors in the instructions to use a PostgreSQL database for Drupal. The fixes include omitted words, wrong indentation, word wrapping, punctuation and grammar.

CommentFileSizeAuthor
#11 1041474-INSTALL-pgsql-mysql-documentation-cleanup.patch4.66 KBTor Arne Thune
#7 1041474-INSTALL-pgsql-documentation-cleanup-3.patch3.67 KBTor Arne Thune
#4 1041474-INSTALL-pgsql-documentation-cleanup-2.patch3.68 KBTor Arne Thune
#2 1041474-INSTALL-pgsql-documentation-cleanup.patch3.7 KBTor Arne Thune
INSTALL-pgsql-documentation-cleanup.patch3.61 KBTor Arne Thune

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks! If you're going to do a cleanup, let's get it really clean. It looks good, but there are a few more items:
- e.g. should always be followed by a comma (it means "for example").
- Our standard for Drupal doc is that in lists, the final "and" should be preceded by a comma:
a, b, and c
There's one needed comma for this in the last paragraph of your patch.
- Can you tell me what this means:
You'll need to execute the SQL below as a superuser (such as a postgres user)
How is any old postgres user necessairly a superuser? Does this make sense to you?

Tor Arne Thune’s picture

Tor Arne Thune commented
Status: Needs work » Needs review
StatusFileSize
new 1041474-INSTALL-pgsql-documentation-cleanup.patch3.7 KB

Thanks for the corrections. Learned two new things.
As for the sentence about the superuser, I think you are right that it is not worded correctly. I'm not very familiar with PostgreSQL, but it must be possible to have PostgreSQL users that are not superusers. In my opinion, the parenthesized text can be omitted. An example can be very helpful, but not when it's wrong. I'm not familiar enough with PostgreSQL to come up with an alternative, but I don't think it's necessary to supply an example, as it's pretty self-explanatory that one needs the correct permissions to create a schema. "You'll need to execute the SQL below as a superuser" insinuates this.

The updated patch corrects the aforementioned errors and removes the parenthesized text.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

I think you forgot to do what you said about the postgres thing (which I thought was a reasonable suggestion):

+   create schemas for you. In fact, the user that Drupal runs as should not be
+   allowed to do this. You'll need to execute the SQL below as a superuser (such
+   as a postgres user), replace 'drupaluser' with the username that Drupal uses
Tor Arne Thune’s picture

Tor Arne Thune commented
Status: Needs work » Needs review
StatusFileSize
new 1041474-INSTALL-pgsql-documentation-cleanup-2.patch3.68 KB

Forgot to remove the parenthesized text. This patch is the correct one. Note to self: Writing about it != writing it.

Tor Arne Thune’s picture

Tor Arne Thune commented

Ah, there we go, you caught it before me.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

That looks better. :) One more thing I just noticed: do you think the 'drupaluser' in the schema section should be replaced by 'username', since that is what is used in the examples above? Other than that I am fine with this going in.

Tor Arne Thune’s picture

Tor Arne Thune commented
StatusFileSize
new 1041474-INSTALL-pgsql-documentation-cleanup-3.patch3.67 KB

I actually thought the same before, so that probably means it's the right choice, as we are two who think this independently. I didn't change it because I thought there was some difference between the username used in the 'Create database user' step in the documentation, and the username mentioned in the 'Create schema' step. As I see now this will obviously be the same user. So, I also think that it's best practice to use the same name for the "placeholders". Patch changed :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Thanks! Now it's good to go.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

If we're going to clean these up in INSTALL.pgsql.txt, it makes sense to make all of the various files consistent.

For example, you've added commas here and there and stuff like that which are not reflected in INSTALL.mysql.txt even though the text is mostly the same.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

OK. Probably should take a look at INSTALL.sqlite.text too.

Tor Arne Thune’s picture

Tor Arne Thune commented
Title: INSTALL.pgsql.txt cleanup of various grammatical errors » INSTALL.pgsql.txt and INSTALL.mysql.txt cleanup of various grammatical errors
Status: Needs work » Needs review
StatusFileSize
new 1041474-INSTALL-pgsql-mysql-documentation-cleanup.patch4.66 KB

Install.mysql.txt cleanup for consistency:

  • set-up > set up
  • e.g. > e.g.,

I couldn't find anything to change for INSTALL.sqlite.txt.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Agreed, I don't see anything else that needs attention in the mysql or sqlite files. This looks good. Thanks!

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Quick fix

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.