Skip to content

fix: clean up CLI usage and README#25

Open
britneywwc wants to merge 5 commits into
mainfrom
cli-flags
Open

fix: clean up CLI usage and README#25
britneywwc wants to merge 5 commits into
mainfrom
cli-flags

Conversation

@britneywwc
Copy link
Copy Markdown
Contributor

Done

  1. Replace CLI steps with config.json file for cleaner setup

QA

  1. Clone this PR
  2. Follow the updated README.md steps
  3. Do a test run on Bauer by modifying config.json

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to simplify Bauer setup by shifting primary configuration from CLI flags to a JSON config.json, and updating documentation accordingly.

Changes:

  • Add GitHub/workflow-related fields to internal/config.Config and enforce github_repo validation.
  • Add --config support in the CLI and load configuration from JSON when provided.
  • Update README.md, config.json, and .gitignore to reflect the new configuration approach.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
internal/config/config.go Adds GitHub/workflow fields to config and updates validation requirements.
config.json Updates default config values and adds new workflow-related keys.
cmd/bauer/main.go Adds --config flag and merges config-file values with CLI flags.
README.md Replaces CLI-heavy usage with config.json-based setup instructions.
.gitignore Ignores local credentials file naming pattern *-credentials.json.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread internal/config/config.go
Comment on lines 83 to +90
if c.DocID == "" {
return errors.New("missing required field: doc_id")
}

if c.GitHubRepo == "" {
return errors.New("missing required field: github_repo")
}

Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

Config.Validate() now requires github_repo, but internal/config.Config is used as the BAU/orchestrator config (e.g., tests and workflow phase 2) where GitHub repo is not part of the required inputs. This makes previously-valid configs fail validation (and will break existing unit tests / JSON loading for non-GitHub runs). Consider keeping GitHub-related fields out of config.Config, or make github_repo conditionally required via a separate validation method (e.g., ValidateForWorkflow vs ValidateForBauer) so orchestration-only configs remain valid.

Copilot uses AI. Check for mistakes.
Comment thread cmd/bauer/main.go
Comment thread cmd/bauer/main.go
Comment thread README.md Outdated
Comment thread config.json Outdated
Copy link
Copy Markdown
Collaborator

@muhammadbassiony muhammadbassiony left a comment

Choose a reason for hiding this comment

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

I can't run the API Im getting the following error:

{"time":"2026-03-16T10:37:00.505563+01:00","level":"INFO","msg":"Copilot client stopped successfully"}
{"time":"2026-03-16T10:37:00.505568+01:00","level":"ERROR","msg":"Failed to start Copilot","error":"failed to start Copilot client: CLI process exited: exit status 1\nfailed to kill CLI process: os: process already finished"}
{"time":"2026-03-16T10:37:00.505581+01:00","level":"ERROR","msg":"job execution failed","error":"failed to start Copilot: failed to start Copilot client: CLI process exited: exit status 1\nfailed to kill CLI process: os: process already finished","requestID":"adcd5afa-211b-11f1-999b-46a16ddf096a"}

although I dont really think this is related to this PR in particular.

I'll approve this for now but I think we have to take a step back and make sure everything is working correctly and both CLI and API modes are supported equally.

There are some comments that are not really related to this PR but are general:

  • why did we remove proper CLI support for the tool? was it not feasible to maintain both approaches in the code base?

Comment thread README.md
Comment on lines 5 to +22
@@ -25,86 +19,65 @@ brew update
brew upgrade bauer
```

N.B. You need to install [Copilot CLI](https://docs.github.com/en/copilot/how-tos/set-up/install-copilot-cli) which is used by Bauer.
N.B. You need to install [Copilot CLI](https://docs.github.com/en/copilot/how-tos/set-up/install-copilot-cli) which is used by Bauer. -->
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.

Why are we removing this section?

Comment thread config.json
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.

Im wondering why we moved to a config.json approach, it will not be transferrable if we want to deploy to a Juju environment and we'll have to switch back to env variables

Comment thread README.md
2. Create a local credentials file: `cp credentials.json bau-credentials.json`
3. Get credentials from Google Cloud service or Bitwarden (internally)
4. Fill up `credentials.json` with Google Cloud credentials (see [Generating Google Cloud credentials](https://developers.google.com/workspace/guides/create-credentials)).
4. Fill up `bau-credentials.json` with Google Cloud credentials (see [Generating Google Cloud credentials](https://developers.google.com/workspace/guides/create-credentials)).
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.

we should probably say to get them from bitwarden instead

Comment thread config.json
{
"doc_id": "1WJ-N_Xkkx4r_6knxW7h200oIDyi4mVMzgh1xYt5xaU0",
"credentials": "bau-test-creds.json",
"doc_id": "",
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.

Not for this PR: we should probably not have a default for this at all, this one in particular should be sent as a param either to the API or CLI

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.

3 participants