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

feat: add table of cli flags to getting started guide #3048

Merged
merged 3 commits into from
May 30, 2023

Conversation

allending313
Copy link
Contributor

  • Identified the issue which this PR solves.
  • Read the CONTRIBUTING document.
  • Code builds clean without any errors or warnings.
  • Added appropriate tests for any new functionality.
  • All new and existing tests passed.
  • Added comments in the code, where necessary.
  • Ran make check to catch common errors. Fixed any that came up.

Description:
Adds table to the getting started markdown with:

  • All available command line flags
  • Corresponding environment variables for each flag
  • Clear and concise descriptions for each flag and variable

Which issue(s) this PR fixes:
Fixes #3023

Screenshots:
Screenshot 2023-05-25 at 11 03 40 AM

@mastercactapus
Copy link
Member

Looks great!

There are a few that are the main ones to be used, can we move those to the top?

  • --db-url and --db-url-next
  • --public-url
  • --data-encryption-key and --data-encryption-key-old

Also because those are listed as the primary ones by this guide:

To run GoAlert, you can start the binary directly, or from a container image. You will need to specify the --db-url, --public-url, and --data-encryption-key you plan to use.

Copy link
Contributor

@andrewbenington andrewbenington left a comment

Choose a reason for hiding this comment

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

LGTM!

@mastercactapus
Copy link
Member

mastercactapus commented May 26, 2023

I like the bold, but maybe --db-url-next and --data-encryption-key-old should not be (still at the top). We don't want to imply they should ever be used except in certain circumstances. Maybe append to the description of each (flag and table) something like "only use this when performing a switchover" and "only necessary when changing --data-encryption-key" respectively.

That way, the bold flags are the ones you need to use always, and the additional context will prevent confusion for new users.

Copy link
Contributor

@andrewbenington andrewbenington left a comment

Choose a reason for hiding this comment

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

👍

@mastercactapus mastercactapus merged commit c96ae6f into master May 30, 2023
@mastercactapus mastercactapus deleted the feat/3023-add-cli-flags branch May 30, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Table of CLI Flags, Environment Variables, and Descriptions to Getting Started Guide
4 participants