Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update bito-cra.ps1 - default branch and acceptable suggestions #46

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Update bito-cra.ps1 - default branch and acceptable suggestions #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5787cf9
Update bito-cra.ps1 - default branch and acceptable suggestions
rishabhbohra-bito Dec 11, 2024
35a4723
Update bito-cra.sh
rishabhbohra-bito Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion cra-scripts/bito-cra.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,12 +404,14 @@ $required_params_cli = @(
)

$optional_params_cli = @(
"acceptable_suggestions_enabled",
"review_comments",
"static_analysis",
"static_analysis_tool",
"linters_feedback",
"secret_scanner_feedback",
"review_scope",
"enable_default_branch",
"exclude_branches",
"exclude_files",
"exclude_draft_pr",
Expand Down Expand Up @@ -438,6 +440,7 @@ $required_params_server = @(
)

$optional_params_server = @(
"acceptable_suggestions_enabled",
"git.provider",
"git.access_token",
"bito_cli.bito.access_key",
Expand All @@ -447,6 +450,7 @@ $optional_params_server = @(
"linters_feedback",
"secret_scanner_feedback",
"review_scope",
"enable_default_branch",
"exclude_branches",
"exclude_files",
"exclude_draft_pr",
Expand Down Expand Up @@ -534,7 +538,7 @@ foreach ($param in $required_params) {
foreach ($param in $optional_params) {
if ($param -eq "dependency_check.snyk_auth_token" -and $props["dependency_check"] -eq "True") {
Ask-For-Param $param $false
} elseif ($param -ne "dependency_check.snyk_auth_token" -and $param -ne "env" -and $param -ne "cli_path" -and $param -ne "output_path" -and $param -ne "static_analysis_tool" -and $param -ne "linters_feedback" -and $param -ne "secret_scanner_feedback" -and $param -ne "git.domain" -and $param -ne "review_scope" -and $param -ne "exclude_branches" -and $param -ne "exclude_files" -and $param -ne "exclude_draft_pr" -and $param -ne "cr_event_type" -and $param -ne "posting_to_pr" -and $param -ne "custom_rules.configured_ws_ids" -and $param -ne "custom_rules.aws_access_key_id" -and $param -ne "custom_rules.aws_secret_access_key" -and $param -ne "custom_rules.region_name" -and $param -ne "custom_rules.bucket_name" -and $param -ne "custom_rules.aes_key" -and $param -ne "code_context_config.partial_timeout" -and $param -ne "code_context_config.max_depth" -and $param -ne "code_context_config.kill_timeout_sec") {
} elseif ($param -ne "acceptable_suggestions_enabled" -and $param -ne "dependency_check.snyk_auth_token" -and $param -ne "env" -and $param -ne "cli_path" -and $param -ne "output_path" -and $param -ne "static_analysis_tool" -and $param -ne "linters_feedback" -and $param -ne "secret_scanner_feedback" -and $param -ne "enable_default_branch" -and $param -ne "git.domain" -and $param -ne "review_scope" -and $param -ne "exclude_branches" -and $param -ne "exclude_files" -and $param -ne "exclude_draft_pr" -and $param -ne "cr_event_type" -and $param -ne "posting_to_pr" -and $param -ne "custom_rules.configured_ws_ids" -and $param -ne "custom_rules.aws_access_key_id" -and $param -ne "custom_rules.aws_secret_access_key" -and $param -ne "custom_rules.region_name" -and $param -ne "custom_rules.bucket_name" -and $param -ne "custom_rules.aes_key" -and $param -ne "code_context_config.partial_timeout" -and $param -ne "code_context_config.max_depth" -and $param -ne "code_context_config.kill_timeout_sec") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider restructuring long condition statement

Consider breaking down the long condition into multiple lines or extracting it into a helper function for better readability. The current line exceeds recommended line length and makes code maintenance difficult.

Code suggestion
Check the AI-generated fix before applying
 @@ -534,11 +534,20 @@
  function Should-Prompt-Parameter($param) {
     $excluded_params = @(
         "acceptable_suggestions_enabled", "dependency_check.snyk_auth_token", "env", "cli_path", 
         "output_path", "static_analysis_tool", "linters_feedback", "secret_scanner_feedback", 
         "enable_default_branch", "git.domain", "review_scope", "exclude_branches", "exclude_files", 
         "exclude_draft_pr", "cr_event_type", "posting_to_pr", "custom_rules.configured_ws_ids", 
         "custom_rules.aws_access_key_id", "custom_rules.aws_secret_access_key", "custom_rules.region_name", 
         "custom_rules.bucket_name", "custom_rules.aes_key", "code_context_config.partial_timeout", 
         "code_context_config.max_depth", "code_context_config.kill_timeout_sec"
     )
     return $excluded_params -notcontains $param
  }

  foreach ($param in $optional_params) {
      if ($param -eq "dependency_check.snyk_auth_token" -and $props["dependency_check"] -eq "True") {
          Ask-For-Param $param $false
 -    } elseif ($param -ne "acceptable_suggestions_enabled" -and $param -ne "dependency_check.snyk_auth_token" -and $param -ne "env" -and $param -ne "cli_path" -and $param -ne "output_path" -and $param -ne "static_analysis_tool" -and $param -ne "linters_feedback" -and $param -ne "secret_scanner_feedback" -and $param -ne "enable_default_branch" -and $param -ne "git.domain" -and $param -ne "review_scope" -and $param -ne "exclude_branches" -and $param -ne "exclude_files" -and $param -ne "exclude_draft_pr" -and $param -ne "cr_event_type" -and $param -ne "posting_to_pr" -and $param -ne "custom_rules.configured_ws_ids"  -and  $param -ne "custom_rules.aws_access_key_id"  -and  $param -ne "custom_rules.aws_secret_access_key"  -and  $param -ne "custom_rules.region_name"  -and  $param -ne "custom_rules.bucket_name"  -and  $param -ne "custom_rules.aes_key"  -and  $param -ne "code_context_config.partial_timeout"  -and  $param -ne "code_context_config.max_depth"  -and  $param -ne "code_context_config.kill_timeout_sec") {
 +    } elseif (Should-Prompt-Parameter $param) {
          Ask-For-Param $param $false
      }

Code Review Run #9f0e9d

Is this a valid issue, or was it incorrectly flagged by the Agent?

  • it was incorrectly flagged

Ask-For-Param $param $false
}
}
Expand Down Expand Up @@ -566,9 +570,15 @@ foreach ($param in $required_params + $bee_params + $optional_params) {
} elseif ($param -eq "secret_scanner_feedback") {
$validated_boolean = Validate-Boolean $props[$param]
$docker_cmd += " --$param=$validated_boolean"
} elseif ($param -eq "acceptable_suggestions_enabled") {
$validated_boolean = Validate-Boolean $props[$param]
$docker_cmd += " --$param=$validated_boolean"
} elseif ($param -eq "review_scope") {
$scopes = $($props[$param]) -replace ',\s*', ','
$docker_cmd += " --$param='[$scopes]'"
} elseif ($param -eq "enable_default_branch") {
$validated_boolean = Validate-Boolean $props[$param]
$docker_cmd += " --$param=$validated_boolean"
} elseif ($param -eq "exclude_branches") {
$docker_cmd += " --exclude_branches='$($props[$param])'"
} elseif ($param -eq "exclude_files") {
Expand Down
12 changes: 11 additions & 1 deletion cra-scripts/bito-cra.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,14 @@ required_params_cli=(
)

optional_params_cli=(
"acceptable_suggestions_enabled"
"review_comments"
"static_analysis"
"static_analysis_tool"
"linters_feedback"
"secret_scanner_feedback"
"review_scope"
"enable_default_branch"
"exclude_branches"
"exclude_files"
"exclude_draft_pr"
Expand Down Expand Up @@ -451,12 +453,14 @@ optional_params_server=(
"git.provider"
"git.access_token"
"bito_cli.bito.access_key"
"acceptable_suggestions_enabled"
"review_comments"
"static_analysis"
"static_analysis_tool"
"linters_feedback"
"secret_scanner_feedback"
"review_scope"
"enable_default_branch"
"exclude_branches"
"exclude_files"
"exclude_draft_pr"
Expand Down Expand Up @@ -542,7 +546,7 @@ done
for param in "${optional_params[@]}"; do
if [ "$param" == "dependency_check.snyk_auth_token" ] && [ "${props["dependency_check"]}" == "True" ]; then
ask_for_param "$param" "False"
elif [ "$param" != "dependency_check.snyk_auth_token" ] && [ "$param" != "env" ] && [ "$param" != "cli_path" ] && [ "$param" != "output_path" ] && [ "$param" != "static_analysis_tool" ] && [ "$param" != "linters_feedback" ] && [ "$param" != "secret_scanner_feedback" ] && [ "$param" != "git.domain" ] && [ "$param" != "review_scope" ] && [ "$param" != "exclude_branches" ] && [ "$param" != "nexus_url" ] && [ "$param" != "exclude_files" ] && [ "$param" != "exclude_draft_pr" ] && [ "$param" != "cr_event_type" ] && [ "$param" != "posting_to_pr" ] && [ "$param" != "custom_rules.configured_ws_ids" ] && [ "$param" != "custom_rules.aws_access_key_id" ] && [ "$param" != "custom_rules.aws_secret_access_key" ] && [ "$param" != "custom_rules.region_name" ] && [ "$param" != "custom_rules.bucket_name" ] && [ "$param" != "custom_rules.aes_key" ] && [ "$param" != "code_context_config.partial_timeout" ] && [ "$param" != "code_context_config.max_depth" ] && [ "$param" != "code_context_config.kill_timeout_sec" ]; then
elif [ "$param" != "acceptable_suggestions_enabled" ] && [ "$param" != "dependency_check.snyk_auth_token" ] && [ "$param" != "env" ] && [ "$param" != "cli_path" ] && [ "$param" != "output_path" ] && [ "$param" != "static_analysis_tool" ] && [ "$param" != "linters_feedback" ] && [ "$param" != "secret_scanner_feedback" ] && [ "$param" != "enable_default_branch" ] && [ "$param" != "git.domain" ] && [ "$param" != "review_scope" ] && [ "$param" != "exclude_branches" ] && [ "$param" != "nexus_url" ] && [ "$param" != "exclude_files" ] && [ "$param" != "exclude_draft_pr" ] && [ "$param" != "cr_event_type" ] && [ "$param" != "posting_to_pr" ] && [ "$param" != "custom_rules.configured_ws_ids" ] && [ "$param" != "custom_rules.aws_access_key_id" ] && [ "$param" != "custom_rules.aws_secret_access_key" ] && [ "$param" != "custom_rules.region_name" ] && [ "$param" != "custom_rules.bucket_name" ] && [ "$param" != "custom_rules.aes_key" ] && [ "$param" != "code_context_config.partial_timeout" ] && [ "$param" != "code_context_config.max_depth" ] && [ "$param" != "code_context_config.kill_timeout_sec" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider refactoring long parameter check condition

The condition check contains a long chain of != comparisons which makes the code hard to maintain and error-prone. Consider using an array or associative array to store these parameter names and using a loop to check if the parameter exists in the array.

Code suggestion
Check the AI-generated fix before applying
 @@ -546,7 +546,19 @@
  for param in "${optional_params[@]}"; do
    if [ "$param" == "dependency_check.snyk_auth_token" ] && [ "${props["dependency_check"]}" == "True" ]; then
        ask_for_param "$param" "False"
 -  elif [ "$param" != "acceptable_suggestions_enabled" ] && [ "$param" != "dependency_check.snyk_auth_token" ] && [ "$param" != "env" ] && [ "$param" != "cli_path" ] && [ "$param" != "output_path" ] && [ "$param" != "static_analysis_tool" ]  && [ "$param" != "linters_feedback" ] && [ "$param" != "secret_scanner_feedback" ] && [ "$param" != "enable_default_branch" ] && [ "$param" != "git.domain" ] && [ "$param" != "review_scope" ] && [ "$param" != "exclude_branches" ] && [ "$param" != "nexus_url" ] && [ "$param" != "exclude_files" ] && [ "$param" != "exclude_draft_pr" ] && [ "$param" != "cr_event_type" ] && [ "$param" != "posting_to_pr" ] && [ "$param" != "custom_rules.configured_ws_ids" ] && [ "$param" != "custom_rules.aws_access_key_id" ] && [ "$param" != "custom_rules.aws_secret_access_key" ] && [ "$param" != "custom_rules.region_name" ] && [ "$param" != "custom_rules.bucket_name" ] && [ "$param" != "custom_rules.aes_key" ] && [ "$param" != "code_context_config.partial_timeout" ] && [ "$param" != "code_context_config.max_depth" ] && [ "$param" != "code_context_config.kill_timeout_sec" ]; then
 +  else
 +    skip_params=(
 +      "acceptable_suggestions_enabled" "dependency_check.snyk_auth_token" "env" "cli_path" "output_path"
 +      "static_analysis_tool" "linters_feedback" "secret_scanner_feedback" "enable_default_branch" "git.domain"
 +      "review_scope" "exclude_branches" "nexus_url" "exclude_files" "exclude_draft_pr" "cr_event_type"
 +      "posting_to_pr" "custom_rules.configured_ws_ids" "custom_rules.aws_access_key_id"
 +      "custom_rules.aws_secret_access_key" "custom_rules.region_name" "custom_rules.bucket_name"
 +      "custom_rules.aes_key" "code_context_config.partial_timeout" "code_context_config.max_depth"
 +      "code_context_config.kill_timeout_sec"
 +    )
 +    skip_param=false
 +    for skip in "${skip_params[@]}"; do
 +      [[ "$param" == "$skip" ]] && skip_param=true && break
 +    done
 +    [[ "$skip_param" == false ]] && ask_for_param "$param" "False"

Code Review Run #9f0e9d

Is this a valid issue, or was it incorrectly flagged by the Agent?

  • it was incorrectly flagged

ask_for_param "$param" "False"
fi
done
Expand Down Expand Up @@ -580,9 +584,15 @@ for param in "${required_params[@]}" "${bee_params[@]}" "${optional_params[@]}";
elif [ "$param" == "secret_scanner_feedback" ]; then
props[$param]=$(validate_boolean "${props[$param]}")
docker_cmd+=" --secret_scanner_feedback=${props[$param]}"
elif [ "$param" == "acceptable_suggestions_enabled" ]; then
props[$param]=$(validate_boolean "${props[$param]}")
docker_cmd+=" --acceptable_suggestions_enabled=${props[$param]}"
Comment on lines +587 to +589
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider consolidating boolean parameter handling

Consider consolidating the boolean parameter handling into a helper function to reduce code duplication. The pattern validate_boolean followed by setting docker command is repeated multiple times. A similar issue was also found in cra-scripts/bito-cra.ps1 (line 570-581).

Code suggestion
Check the AI-generated fix before applying
 @@ -584,15 +584,7 @@
  function handle_boolean_param() {
    local param=$1
    local value=$2
    props[$param]=$(validate_boolean "$value")
    docker_cmd+=" --$param=${props[$param]}"
  }

      elif [ "$param" == "secret_scanner_feedback" ]; then
 -        props[$param]=$(validate_boolean "${props[$param]}")
 -        docker_cmd+=" --secret_scanner_feedback=${props[$param]}"
 -    elif [ "$param" == "acceptable_suggestions_enabled" ]; then
 -        props[$param]=$(validate_boolean "${props[$param]}")
 -        docker_cmd+=" --acceptable_suggestions_enabled=${props[$param]}"
 +        handle_boolean_param "$param" "${props[$param]}"
 +    elif [ "$param" == "acceptable_suggestions_enabled" ]; then
 +        handle_boolean_param "$param" "${props[$param]}"

Code Review Run #9f0e9d

Is this a valid issue, or was it incorrectly flagged by the Agent?

  • it was incorrectly flagged

elif [ "$param" == "review_scope" ]; then
scopes=$(echo ${props[$param]} | sed 's/, */,/g')
docker_cmd+=" --review_scope='[$scopes]'"
elif [ "$param" == "enable_default_branch" ]; then
props[$param]=$(validate_boolean "${props[$param]}")
docker_cmd+=" --enable_default_branch=${props[$param]}"
elif [ "$param" == "exclude_branches" ]; then
docker_cmd+=" --exclude_branches='${props[$param]}'"
elif [ "$param" == "exclude_files" ]; then
Expand Down