Skip to content
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

Updates to documentation #3457

Merged
merged 3 commits into from Feb 28, 2024
Merged

Updates to documentation #3457

merged 3 commits into from Feb 28, 2024

Conversation

kgodey
Copy link
Contributor

@kgodey kgodey commented Feb 26, 2024

Fixes #3428

This PR makes the following changes:

  • Minor language updates to the docs homepage.
  • Removed the "Connect to an external database server" page entirely, see below.

Reasoning for page removal

The page (see live version) attempted to document two things:

(1) How to set up a Mathesar internal database if you're connecting to an external DB

These steps were out of date anyway, since they referred to configuring the DB via environment variables. Also, it is my understanding that we don't need to perform any steps after the new DB connection UI work in 0.1.4 and that we'll create the internal database automatically. So I removed it.

(2) How to configure Mathesar to point at a database on the host server if you're using Docker

My first instinct was to move this to the "Optional configurations" section of the Docker compose installation, and I spent some time doing that and making the documentation more concise. Then I realized that

  • none of the documentation was about Mathesar, it's all about configuring Postgres and Docker and is probably outside the scope of our docs.
  • we only document one way of doing this and present it as "the way", but there seem to be multiple approaches that can be taken to solve the problem of connecting to a Postgres database on a Docker host
  • we're presuming usage of our old Docker compose file

It didn't seem like it was worth cleaning up or having this documentation, so I removed it.

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the develop branch of the repository
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no
    visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@kgodey kgodey added this to the v0.1.5 milestone Feb 26, 2024
@kgodey
Copy link
Contributor Author

kgodey commented Feb 26, 2024

@mathemancer please double check my reasoning here, that's the main thing I'm asking you to review.

@seancolsen also added you as a reviewer in case you wanted to look at the index page changes.

Copy link
Contributor

@mathemancer mathemancer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with (2), but (1) isn't correct. If you want to use a specific DB server for the internal (Django) DB, you still set that up using environment variables. This section should perhaps be clarified, but it shouldn't be removed.

@kgodey
Copy link
Contributor Author

kgodey commented Feb 28, 2024

@mathemancer Thanks for the feedback. As discussed on our call yesterday, I've:

  • Added back the external Postgres server docs, but to the Docker Compose page.
  • Added a note to the env variables docs about Caddy.

I decided not to change the build from source page because Set up the database > Step 3 already documents creating the internal database, and Install the Mathesar application > Step 2 also documents setting the environment variables. I'm not sure what we could add there.

@seancolsen
Copy link
Contributor

@kgodey I think this PR should target the 0.1.5 branch now, right?

@kgodey kgodey changed the base branch from develop to 0.1.5 February 28, 2024 15:59
Copy link
Contributor

@mathemancer mathemancer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@kgodey kgodey merged commit a66821a into 0.1.5 Feb 28, 2024
22 checks passed
@kgodey kgodey deleted the docs_updates branch February 28, 2024 16:56
seancolsen added a commit that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update "Connect to an external database server" docs
3 participants