Skip to content

Conversation

@ghinks
Copy link
Owner

@ghinks ghinks commented Oct 25, 2025

Summary

  • ensure CLI arguments override configuration defaults where appropriate and tighten TOML parsing validation
  • prevent configuration files from enabling both plotting modes at once and guard repository fetching against missing organizations
  • expand CLI parser coverage for plotting flags and cache overrides

Testing

  • poetry run ruff check .
  • poetry run mypy reviewtally/
  • poetry run pytest

https://chatgpt.com/codex/tasks/task_e_68fd08f326d8833088bbb235c9cd1115

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +363 to +376
config_repositories: list[str] | None = None
if "repositories" in config_data:
try:
config_repositories = _parse_repositories(
config_data.get("repositories"),
)
except ConfigError as exc:
print(exc) # noqa: T201
sys.exit(1)

org_name = args.org if args.org is not None else config_data.get("org")
if not org_name and not config_repositories:
print("Error: Provide an organization or repositories to analyze") # noqa: T201
sys.exit(1)

Choose a reason for hiding this comment

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

P1 Badge Allow cache-only commands without repository arguments

The new config validation requires either an organization or a repositories list to be supplied. This check runs before the CLI looks at --clear-cache, --clear-expired-cache, or --cache-stats, so commands like review-tally --clear-cache now exit with "Error: Provide an organization or repositories to analyze" even though cache management doesn’t need repository context. Previously these operations worked without an org, and the early return in main() still bypasses repository fetching. Consider skipping this validation when any cache-maintenance flag is set so users can manage the cache without supplying irrelevant org information.

Useful? React with 👍 / 👎.

@ghinks ghinks changed the title fix: resolve config plot conflicts add configuration file support Oct 26, 2025
@ghinks
Copy link
Owner Author

ghinks commented Oct 26, 2025

closing PR due to merge conflicts.

@ghinks ghinks closed this Oct 26, 2025
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.

2 participants