Skip to content

Set up .env.default #21#22

Merged
evert merged 1 commit intomainfrom
sk/21/set-up-env-defaults
Sep 28, 2022
Merged

Set up .env.default #21#22
evert merged 1 commit intomainfrom
sk/21/set-up-env-defaults

Conversation

@syedfkabir
Copy link
Contributor

Resolves #21

Changes

  • added 2 packages (dotenv and dotenv-defaults)
  • updated the knexfile

Testing

Tested it by navigating the HAL browser, nothing broke!

Copy link

@ikbensiep ikbensiep left a comment

Choose a reason for hiding this comment

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

lgtm!

PUBLIC_URI="http://localhost:8902/"

AUTH_API_URI=http://localhost:8531
OAUTH2_CLIENT_ID=
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can probably pick a good default for this?

"@curveball/router": "^0.4.1",
"@curveball/validator": "^0.9.0",
"dotenv": "^16.0.2",
"dotenv-defaults": "^5.0.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I regret adding dotenv-defaults to our other packages. I wonder if we should just remove these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not opposed to it!

@@ -1,3 +1,7 @@
/* eslint-disable @typescript-eslint/no-var-requires */
require('dotenv').config();
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's possible now to do this with import and not require!

user: 'tt',
password: 'password',
database: 'tt',
host: process.env.MYSQL_HOST,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another thing that's worth testing is migrations. Do they run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they do not! hmmm

@evert evert merged commit a85fb59 into main Sep 28, 2022
@evert evert deleted the sk/21/set-up-env-defaults branch September 28, 2022 15:12
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.

Set up .env.default

4 participants