Skip to content

CLI QOL; docs update#55

Merged
krashanoff merged 4 commits into
masterfrom
krashanoff/cli-qol
May 24, 2021
Merged

CLI QOL; docs update#55
krashanoff merged 4 commits into
masterfrom
krashanoff/cli-qol

Conversation

@krashanoff

Copy link
Copy Markdown
Contributor

Short PR to address a few issues that new members were encountering during onboarding to the backend:

  • Multiline string literals in .env were improperly parsed. To address this, we have temporarily switched our godotenv to x1unix's fork. For further reading on the issue and why we are using a fork rather than waiting for the changes to be merged, see Doesn't support multiline vars joho/godotenv#64 and Add multi-line values support joho/godotenv#118.
  • CLI couldn't load .env files because our key names were inconsistent. Addressed this by matching them. See server.go#L43 and L128.
  • .env options take precedence over CLI options. This should be the default, since most of our developers are simply running ./bin/tlabe. These changes appear in server.go#L88:93

Before this PR is ready for review, I plan on also updating our README.md to reflect the new project structure.

@krashanoff krashanoff marked this pull request as ready for review May 13, 2021 09:24
@krashanoff krashanoff requested a review from timothycpoon May 13, 2021 09:27
@krashanoff krashanoff added bug Something isn't working documentation Improvements or additions to documentation labels May 13, 2021

@timothycpoon timothycpoon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. One small thing which shouldn't be an issue; I think that if a .env file doesn't exist, the program just crashes.

@krashanoff krashanoff requested a review from timothycpoon May 22, 2021 09:29
@krashanoff

Copy link
Copy Markdown
Contributor Author

I'll leave this open until midnight tonight. I removed the code that kills the program if a .env file does not exist.

@timothycpoon timothycpoon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@krashanoff krashanoff merged commit 558a0d8 into master May 24, 2021
@krashanoff krashanoff deleted the krashanoff/cli-qol branch May 24, 2021 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants