Skip to content

chore: rearrange .env files#49

Merged
devonzara merged 10 commits into
mainfrom
dz-rearrange-env-files
Mar 29, 2023
Merged

chore: rearrange .env files#49
devonzara merged 10 commits into
mainfrom
dz-rearrange-env-files

Conversation

@devonzara

Copy link
Copy Markdown
Collaborator

Closes N/A

Description

We were operating under the understanding that the .env.<environment> files set the defaults for each environment and the .env file would override. Based on dotenv-flow's documentation, that is the opposite of how it works.

This change introduces a .env.defaults to initialize default values, users are still instructed to create a .env for configuring secrets, and the environment-specific .env.<environment> files will have the highest priority and overwrite all other files.

Testing

Ensure that the project starts and the tests still pass.

Type of change

Please remove all except for everything applicable, and then remove this line.

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

N/A

Checklist:

  • Changes have new/updated automated tests, if applicable
  • Changes have new/updated docs, if applicable
  • I have performed a self-review of my own code
  • I have added comments on any new, hard-to-understand code
  • Changes have been manually tested
  • Changes generate no new errors or warnings

@timmyichen timmyichen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for making this change!

Comment thread .env.sample

@MainlyColors MainlyColors left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Documenting Thoughts:
Thought we needed docker-compose.test.yml file but looks like when no -f option is given to docker-compose then docker will look for the default docker-compose.yml file and we were already creating a new test DB with the createDb() method in server/test/server.ts TestServer class so there is no reason to have separate docker-compose.test.yml file as it wasn't being used

@timmyichen

timmyichen commented Mar 24, 2023

Copy link
Copy Markdown
Collaborator

Documenting Thoughts: Thought we needed docker-compose.test.yml file but looks like when no -f option is given to docker-compose then docker will look for the default docker-compose.yml file and we were already creating a new test DB with the createDb() method in server/test/server.ts TestServer class so there is no reason to have separate docker-compose.test.yml file as it wasn't being used

ah it's being used in the github action here

I think @MatthewBozin might remember better why exactly it was needed but I remember it breaking with the default docker-compose

@Caleb-Cohen Caleb-Cohen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, Devon never does wrong.

@devonzara devonzara requested a review from timmyichen March 27, 2023 03:51
@devonzara devonzara merged commit 6359c95 into main Mar 29, 2023
@devonzara devonzara deleted the dz-rearrange-env-files branch March 29, 2023 20:06
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.

4 participants