Skip to content

Conversation

fernandosantos-br
Copy link
Collaborator

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 10, 2025 04:46
Copilot

This comment was marked as outdated.

Copilot

This comment was marked as outdated.

Copy link

@Copilot 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

Adds support for configurable build modes, query files, and JSON-based monorepo configuration to the CodeQL wrapper, and updates related tests and CLI behavior.

  • Introduces build_mode and queries parameters throughout the runner and use case layers.
  • Adds executable‐permission handling for build scripts and emits command lines via click.echo in the runner.
  • Implements .codeql.json–driven monorepo analysis and strengthens CLI argument validation.

Reviewed Changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_codeql_runner.py Updated expected CLI args to include --build-mode/--overwrite flags
tests/test_codeql_analysis.py Switched to config-mode tests for monorepo, adjusted patch scopes
tests/test_cli.py Clarified exit-code assertion wording
src/codeql_wrapper/infrastructure/codeql_runner.py Added build_mode and queries flags, script-permission logic, and click.echo
src/codeql_wrapper/domain/use_cases/codeql_analysis_use_case.py Refactored monorepo to read .codeql.json, added config-based project processing
src/codeql_wrapper/domain/entities/codeql_analysis.py Extended CodeQLAnalysisRequest with build_mode, build_script, queries
src/codeql_wrapper/cli.py Made repository_path required, custom validation, and .codeql.json detection log
Comments suppressed due to low confidence (7)

src/codeql_wrapper/infrastructure/codeql_runner.py:116

  • The database creation always appends --overwrite with no option to disable it. If this is intentional, document it; otherwise provide a flag to control overwriting behavior.
        args.append("--overwrite")

src/codeql_wrapper/domain/use_cases/codeql_analysis_use_case.py:59

  • The code uses json.load but the json module is not imported in this file, which will raise a NameError. Please add import json at the top.
                        config_data = json.load(f)

src/codeql_wrapper/domain/use_cases/codeql_analysis_use_case.py:130

  • This line references os.cpu_count() but the os module is not imported. Please add import os at the top of the file.
        max_workers = min(os.cpu_count() or 1, self.DEFAULT_MAX_WORKERS)

src/codeql_wrapper/cli.py:148

  • The log claims the repository path is switched to the current directory, but the code does not actually modify repository_path. Either remove or correct this log message or update the code to change the path.
            )

src/codeql_wrapper/infrastructure/codeql_runner.py:296

  • [nitpick] Printing command lines via click.echo mixes CLI output with the runner logic. Consider using the logger (e.g., self.logger.debug) for consistency and testability.
        click.echo(f"Executing CodeQL command: {' '.join(command)}")

src/codeql_wrapper/cli.py:141

  • [nitpick] Using sys.exit in a Click command bypasses Click's error handling. Consider raising a click.ClickException or using ctx.abort() to maintain consistent exit codes and messages.
            sys.exit(1)

tests/test_codeql_analysis.py:802

  • [nitpick] Patching pathlib.Path.exists at the module level affects all Path calls in the test. Consider using patch.object on the specific Path instance or method to limit scope and avoid unintended interactions.
        ):

@fernandosantos-br fernandosantos-br merged commit db8262b into main Jul 10, 2025
7 checks passed
@fernandosantos-br fernandosantos-br deleted the develop-root-codeql-yml branch July 10, 2025 22:19
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