Skip to content

Conversation

@BenjiMilan
Copy link
Contributor

Currently we have sc clone add --no-repo-verify if you provide repo_url or repo_rev. However with our current setup even without either of these we need --no-repo-verify as our repo entrypoint applies them anyway. Therefore we need to always apply no-repo-verify

@BenjiMilan BenjiMilan requested a review from TB-1993 November 5, 2025 11:08
@BenjiMilan BenjiMilan self-assigned this Nov 5, 2025
Copilot AI review requested due to automatic review settings November 5, 2025 11:08
Copy link

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 simplifies the no_repo_verify parameter logic in the _init_repo method by removing conditional logic and always setting it to True.

Key Changes:

  • Removed the conditional assignment of no_repo_verify based on repo_url or repo_rev configuration
  • Changed the no_repo_verify parameter to always be True when calling RepoLibrary.init()
Comments suppressed due to low confidence (1)

src/sc/clone/cloners/repo_cloner.py:30

  • The documentation for no_repo_verify in RepoClonerConfig indicates it should be set to true when using a custom repo_url, but this parameter is not actually exposed as a configurable field in the Pydantic model (line 42 shows verify: bool but no no_repo_verify field). With the code now always passing no_repo_verify = True, this documentation is misleading and should either be updated to reflect the actual behavior or the field should be added to the config if it needs to be user-configurable.
        no_repo_verify (bool): Stops repo checking the validity of the version of repo
            used. Should be set to true if using a custom repo_url.

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

Copilot AI review requested due to automatic review settings November 5, 2025 11:34
@BenjiMilan BenjiMilan force-pushed the feature/BWDO-522_always_no_repo_verify branch from f7e9b65 to d6379f7 Compare November 5, 2025 11:34
Copy link

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

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


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

@BenjiMilan BenjiMilan force-pushed the feature/BWDO-522_always_no_repo_verify branch from d6379f7 to 760c3cd Compare November 5, 2025 11:46
@BenjiMilan BenjiMilan requested a review from Copilot November 5, 2025 11:49
Copy link

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

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


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

cloner_config.manifest = cli_overrides.get("manifest")

if cli_overrides.get("verify"):
logger.info("Option [--verify]: Run repo hooks without verification")
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The log message is misleading. Based on the code context and CLI help text, the --verify option runs post-sync hooks WITHOUT prompts (i.e., automatically), not 'without verification'. The message should say 'Run repo hooks without prompts' to match the CLI help text in clone_cli.py.

Suggested change
logger.info("Option [--verify]: Run repo hooks without verification")
logger.info("Option [--verify]: Run repo hooks without prompts")

Copilot uses AI. Check for mistakes.
…dkcentral/sc into feature/BWDO-522_always_no_repo_verify
@BenjiMilan BenjiMilan force-pushed the feature/BWDO-522_always_no_repo_verify branch from 760c3cd to 7cd115e Compare November 5, 2025 11:51
Copy link

@TB-1993 TB-1993 left a comment

Choose a reason for hiding this comment

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

Looks good, ship it!

@BenjiMilan BenjiMilan merged commit 51f8d3b into develop Nov 5, 2025
5 checks passed
@BenjiMilan BenjiMilan deleted the feature/BWDO-522_always_no_repo_verify branch November 5, 2025 13:39
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants