-
Notifications
You must be signed in to change notification settings - Fork 490
docs: explain PGDATA new values #431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
and fix markdown warnings in README
README.md
Outdated
|
||
In order to run a basic container capable of serving a PostGIS-enabled database, start a container as follows: | ||
|
||
```sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These code blocks seem to be rendering incorrectly. Maybe because they are indented?
README.md
Outdated
* Note that ports which are not bound to the host (i.e., `-p 5432:5432` instead of `-p 127.0.0.1:5432:5432`) will be accessible from the outside. This also applies if you configured UFW to block this specific port, as Docker manages its own iptables rules. ( [Read More](https://docs.docker.com/network/iptables/) ) | ||
|
||
#### Recomendations: | ||
### Recomendations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo fix opportunity: I believe "Recomendations" should be spelled "Recommendations"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this and cleaning up. I've added two comments as requests for changes.
Thank you very much for the PR! I have two small pieces of feedback: 1.) For Postgres >=18 it is probably better to recommend using I tested with:
and inside the container the data is automatically placed in the new path: On the host I can also see the files created under
So it looks like we should really follow the upstream suggestion, since that path is version-independent. I usually refresh the From the above, my first remark is probably the more important one. |
If you don’t mind, I’d like to add a few small changes. |
- improve update.sh - with automatic README.md update. ( add new tool apply-readme.md )
WIP Still working on this ; a few small formatting details are not perfect yet. here’s the current status:
I still see a few minor issues, but I’ll polish them soon and hand it over for review. |
READY to review:
|
Hi @thomasboussekey, I made some significant changes, and here is the proposed new README.md: Please take a look when you have time. I think it’s a bit better now, but there’s still room for improvement. One key takeaway is that Docker Hub’s Markdown (https://hub.docker.com/r/postgis/postgis) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine!
Great work @ImreSamu
and fix markdown warnings in README