-
Notifications
You must be signed in to change notification settings - Fork 80
feat(cdr): Docs for remote postgresql C4D #818
Conversation
|
This pull request has been linked to Shortcut Story #20464: C4D remote postgres database. |
|
✨ Coder.com for PR #818 deployed! It will be updated on every commit.
|
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.
Looking good
setup/docker.md
Outdated
| database: | ||
|
|
||
| ```bash | ||
| docker run --rm -it -p 7080:7080 \ |
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.
I think when someone gets to this point, they may want to use Docker Compose to configure things in a file, rather than passing everything in through the command line. Maybe that can be considered in a separate change though.
Since this code block is part of the second list item, I think it should be indented so that it's kept together
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.
That is true. I was thinking of showing the minimum way to do it, but I guess the way we document is how it will be done.
I can change this.
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.
@jawnsy thinking about this again. If I add docker-compose that does add a dependency.
I am ok with keeping this as is
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.
it does add a dependency, but I think examples should be exemplary - that is, examples should be the "recommended" approach that people should use. if we're specifying things as you do here, then what you're implicitly saying is that we recommend people manage Coder this way, and I don't think that's the case...
|
merging into release branch, but am open to updates as needed/wanted |
@khorne3 we should wait until 1.28 to put this in. It works in 1.27, but has some quirks I fixed.