Skip to content

Address PR #75 review feedback: env var support, priority names, and whitespace - #79

Merged
tim-dickey merged 4 commits into
fix/test-unicode-encodingfrom
copilot/sub-pr-75
Feb 12, 2026
Merged

Address PR #75 review feedback: env var support, priority names, and whitespace#79
tim-dickey merged 4 commits into
fix/test-unicode-encodingfrom
copilot/sub-pr-75

Conversation

Copilot AI commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Addresses four code review comments from PR #75 regarding issue creation scripts, tracking documentation, and code quality.

Changes

  • Robust environment variable handling: create-github-issues.py now reads GITHUB_REPOSITORY from environment, normalizes with .strip(), and falls back to "tim-dickey/trivia-app" when unset, empty, or whitespace-only, preventing gh issue create failures with invalid repo values

  • Priority name clarity: Updated code-review-issues-tracking.md to display "Critical" and "High" instead of "(unknown)" for P0/P1 sections

  • Complete issue tracking: Added explanatory note documenting that P0-3 (WebSocket Infrastructure) from source JSON was not created during initial issue creation run

  • Trailing whitespace: Removed trailing spaces from backend/requirements.txt lines 25-26

# Before
REPO = "tim-dickey/trivia-app"

# After
REPO = (os.environ.get("GITHUB_REPOSITORY") or "").strip() or "tim-dickey/trivia-app"

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits February 12, 2026 11:06
- Make create-github-issues.py read GITHUB_REPOSITORY env var with fallback
- Update tracking markdown with proper priority names (Critical, High)
- Add missing P0-3 WebSocket infrastructure issue to tracking
- Remove trailing whitespace from requirements.txt

Co-authored-by: tim-dickey <80638631+tim-dickey@users.noreply.github.com>
Replace TBD placeholder with explanatory note about P0-3 not being created,
avoiding confusion in the checklist tracking

Co-authored-by: tim-dickey <80638631+tim-dickey@users.noreply.github.com>
Copilot AI changed the title [WIP] Standardize on venv for Python virtual environments Address PR #75 review feedback: env var support, priority names, and whitespace Feb 12, 2026
Copilot AI requested a review from tim-dickey February 12, 2026 11:09
@tim-dickey
tim-dickey marked this pull request as ready for review February 12, 2026 11:13
Copilot AI review requested due to automatic review settings February 12, 2026 11:13

Copilot AI 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.

Pull request overview

Updates the issue-creation tooling and tracking docs to address PR #75 feedback, improving fork/CI compatibility and clarifying priority labeling in generated artifacts.

Changes:

  • Read GITHUB_REPOSITORY from the environment in create-github-issues.py (fallback to tim-dickey/trivia-app).
  • Clarify P0/P1 priority names in the tracking markdown and add a note about the missing P0-3 issue.
  • Remove trailing whitespace from two backend/requirements.txt entries.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
scripts/create-github-issues.py Adds env-var driven repo selection for gh issue create runs (supports forks/wrappers).
backend/requirements.txt Removes trailing spaces on two dependency lines.
_bmad-output/implementation-artifacts/code-review-issues-tracking.md Replaces “(unknown)” with “Critical/High” and documents the uncreated P0-3 item.

Comment thread scripts/create-github-issues.py Outdated
from typing import List, Dict, Tuple, Optional

REPO = "tim-dickey/trivia-app"
REPO = os.environ.get("GITHUB_REPOSITORY", "tim-dickey/trivia-app")

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

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

os.environ.get("GITHUB_REPOSITORY", ...) only falls back when the var is unset, not when it's set to an empty/whitespace value. If GITHUB_REPOSITORY is exported but empty, gh issue create --repo "" will fail. Consider normalizing (e.g., .strip()) and falling back when the value is falsy.

Suggested change
REPO = os.environ.get("GITHUB_REPOSITORY", "tim-dickey/trivia-app")
REPO = (os.environ.get("GITHUB_REPOSITORY") or "").strip() or "tim-dickey/trivia-app"

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied the suggestion in commit fdac3c5. The code now normalizes the env var with .strip() and falls back to the default when the value is unset, empty, or whitespace-only.

Tested all scenarios:

  • Valid value → uses it
  • Unset → default
  • Empty string → default
  • Whitespace → default

@tim-dickey

Copy link
Copy Markdown
Owner

@copilot apply changes based on the comments in this thread

Normalize env var value with .strip() and fall back to default when empty or
whitespace-only, preventing gh issue create failures with empty --repo flag

Co-authored-by: tim-dickey <80638631+tim-dickey@users.noreply.github.com>

Copilot AI commented Feb 12, 2026

Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

Applied the review feedback in commit fdac3c5. The GITHUB_REPOSITORY env var handling is now more robust, normalizing empty/whitespace values and falling back to the default to prevent gh issue create failures.

@tim-dickey
tim-dickey merged commit f1ae289 into fix/test-unicode-encoding Feb 12, 2026
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