chore(dx): Improved DX for contributors to help get started

[coltonehrman]

Why?

I made this PR to ease the initial setup of the repo locally, as the code was not configured correctly to get started and was missing a few things. This should help anyone else who's trying to setup locally and contribute :)

Chores

• Made the DB config more configurable to allow for localhost
• Added info to the README to guide the user on setting up DB locally

[labrocadabro]

Thank you so much for this!

I'd like to make a few changes:

1) in the .env, I think it's best to have a DB_LOCAL variable set to true or false. This will make it easier to separate the two connection cases of localhost and Atlas/remote. Some people may want to use their own Atlas account during development, so it's not just about production.

I'm thinking of something along the lines of:

const isLocal = process.env.DB_LOCAL;
if (isLocal.toLowerCase() === "true") {
// build and return localhost URI
} else {
// build and return remote URI
}

2) Instead of DB_CLUSTER, let's go with DB_LOCAL_PORT and DB_REMOTE_URI (with a commented example of what that would look like eg cluster0.58qh2.mongodb.net). This makes it the most flexible, so if someone happens to be using a remote connection string that isn't Atlas, or a localhost port other than 27017, it can still work.

3) I'd prefer to leave fields blank in the .env.example unless they're defaults that are unlikely to be changed (like the localhost port). When you're scanning the file, that makes it a little more obvious what you need to fill in.

4) Can you make the local DB instructions with two options, one with Docker and one without? Not everyone can run Docker - I did 100Devs on a 12 year old computer that couldn't even install Docker Desktop. 😄

5) Please export connectDB and remove the export default from config/db.js

[coltonehrman]

1. in the .env, I think it's best to have a DB_LOCAL variable set to true or false. This will make it easier to separate the two connection cases of localhost and Atlas/remote. Some people may want to use their own Atlas account during development, so it's not just about production.

That makes sense. After thinking about it a bit. I'm wondering if we should just have a single env variable MONGODB_URI to set the whole string instead? I feel like this would be easier to control and maintain. Let me know what you think and I can update the PR.

[labrocadabro]

You know, you're probably right. Let's go with that, it makes things far simpler. 👍

On Fri, Dec 29, 2023 at 11:25 PM Colton Ehrman @.***> wrote:

1. in the .env, I think it's best to have a DB_LOCAL variable set to true or false. This will make it easier to separate the two connection cases of localhost and Atlas/remote. Some people may want to use their own Atlas account during development, so it's not just about production.

That makes sense. After thinking about it a bit. I'm wondering if we should just have a single env variable MONGODB_URI to set the whole string instead? I feel like this would be easier to control and maintain. Let me know what you think and I can update the PR.

— Reply to this email directly, view it on GitHub https://github.com/labrocadabro/communitytaught/pull/30#issuecomment-1872439889, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMGUD6JVKOCEZSU36PWR53YL6CSXAVCNFSM6AAAAABBCTTIJGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZSGQZTSOBYHE . You are receiving this because you commented.Message ID: @.***>

[coltonehrman]

You know, you're probably right. Let's go with that, it makes things far simpler. 👍 … On Fri, Dec 29, 2023 at 11:25 PM Colton Ehrman @.> wrote: 1. in the .env, I think it's best to have a DB_LOCAL variable set to true or false. This will make it easier to separate the two connection cases of localhost and Atlas/remote. Some people may want to use their own Atlas account during development, so it's not just about production. That makes sense. After thinking about it a bit. I'm wondering if we should just have a single env variable MONGODB_URI to set the whole string instead? I feel like this would be easier to control and maintain. Let me know what you think and I can update the PR. — Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMGUD6JVKOCEZSU36PWR53YL6CSXAVCNFSM6AAAAABBCTTIJGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZSGQZTSOBYHE . You are receiving this because you commented.Message ID: @.>

I have updated :)

[coltonehrman]

@labrocadabro Do you plan on merging this? It's not easy to make changes without these code changes merged. I have to "hack" the DB url otherwise.

[labrocadabro]

I apologize, this is a more complex issue, so I've been punting on it. It also just registered that you are connecting through localhost and therefore this change is urgent for you, sorry!

I'd like to move the db connection documentation from the readme to a separate file (we eventually need proper docs), which you can just link from the readme. This will keep it from getting too long and not everyone needs to work with MongoDB locally. I've also made changes to the readme recently, which you'll need to take into account.

If you can, instead of having a default URI for MONGODB_URI, just put two comments above it, one as an example of a localhost connection string, and one as an example of a remote (Atlas) connection.

We don't really need the function getDBUri anymore, we can just use process.env.MONGODB_URI directly.

Once these changes are implemented, I can merge this one.

[labrocadabro]

In the interest of having this complete so you (and anyone else using localhost) can get this working, I've made the edits myself. I didn't include the documentation, which is now probably more appropriate as part of #44, which @KhuloodHassan is working on. I've made them aware of the documentation you've written so it can be incorporated.

As that should take care of everything that this PR addresses, I'm closing this one.