Skip to content

Follow-up: remove unused get_commands cwd parameter#20

Merged
badMade merged 1 commit into
performance-optimize-get-command-1051592804651335151from
badmade/github-mention-optimize-get_command-with-o1-lookup-dictio-r55w0j
Apr 20, 2026
Merged

Follow-up: remove unused get_commands cwd parameter#20
badMade merged 1 commit into
performance-optimize-get-command-1051592804651335151from
badmade/github-mention-optimize-get_command-with-o1-lookup-dictio-r55w0j

Conversation

@badMade

@badMade badMade commented Apr 16, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Address review feedback by removing the unused cwd parameter from get_commands to simplify the function signature and avoid confusion.

Description

  • Remove the cwd: str | None = None argument from get_commands in src/commands.py, preserving existing filtering logic and return type.

Testing

  • Ran pytest -q in a plain environment which failed due to import path setup (ModuleNotFoundError: No module named 'src').
  • Ran PYTHONPATH=. pytest -q and all tests passed (22 passed).

Codex Task

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request removes the unused cwd parameter from the get_commands function in src/commands.py. The reviewer suggests converting the remaining boolean parameters into keyword-only arguments to improve code readability and prevent potential misuse from positional arguments.

Comment thread src/commands.py
@badMade badMade merged commit fb05d80 into performance-optimize-get-command-1051592804651335151 Apr 20, 2026
@badMade badMade deleted the badmade/github-mention-optimize-get_command-with-o1-lookup-dictio-r55w0j branch April 20, 2026 01:29
@badMade

badMade commented Apr 22, 2026

Copy link
Copy Markdown
Owner Author

@copilot code review

@badMade

badMade commented Apr 22, 2026

Copy link
Copy Markdown
Owner Author

@claude code review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Removes an unused cwd parameter from get_commands to simplify the API and eliminate confusion about an argument that had no effect.

Changes:

  • Drop cwd: str | None = None from get_commands in src/commands.py.
  • Keep existing command filtering behavior and return type unchanged.

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

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