Skip to content

Update PR Action Handling Logic in GitHub Action Runner #1231

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

Jump to bottom
Merged
merged 5 commits into from
Sep 15, 2024
Merged

Update PR Action Handling Logic in GitHub Action Runner #1231

merged 5 commits into from
Sep 15, 2024

Conversation

matanbaruch
Copy link
Contributor

@matanbaruch matanbaruch commented Sep 15, 2024 •
edited
Loading

PR Type

Enhancement

Description

  • Implemented configurable PR actions in the GitHub Action runner:
    • Replaced hardcoded PR actions with a list retrieved from settings
    • Added GITHUB_ACTION_CONFIG.PR_ACTIONS configuration option
    • Default actions include: 'opened', 'reopened', 'ready_for_review', 'review_requested'
  • Updated configuration.toml to include the new pr_actions option (commented out)
  • Improved flexibility by allowing users to customize which PR actions trigger the GitHub Action
  • Maintained backward compatibility with existing configuration options

Changes walkthrough 📝

Relevant files
Enhancement
github_action_runner.py
Implement configurable PR actions in GitHub Action runner

pr_agent/servers/github_action_runner.py

  • Replaced hardcoded PR actions with configurable list from settings
  • Added retrieval of PR actions from GITHUB_ACTION_CONFIG.PR_ACTIONS
  • Updated condition to check if the action is in the retrieved PR
    actions list
    		•  +5/-1     
    Configuration changes
    configuration.toml
    Add PR actions configuration option in TOML file                 

    pr_agent/settings/configuration.toml

  • Added commented-out pr_actions configuration option under
    [github_action_config]
  • Provided default values for PR actions: 'opened', 'reopened',
    'ready_for_review', 'review_requested'
    		•  +1/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 95
    🧪 No relevant tests
    🔒 No security concerns identified
    🔀 No multiple PR themes
    ⚡ Key issues to review

    Default Value Handling
    The default value for pr_actions is hardcoded in the get_settings() call. Consider moving this default to a constant or configuration file for better maintainability.

    @qodo-merge-pro Qodo Merge Pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Performance
    Convert the list of actions to a set for faster membership testing

    Consider using a set instead of a list for pr_actions to improve lookup performance
    when checking if an action is in pr_actions.

    pr_agent/servers/github_action_runner.py [88-90]

    -# Retrieve the list of actions from the configuration
-pr_actions = get_settings().get("GITHUB_ACTION_CONFIG.PR_ACTIONS", ["opened", "reopened", "ready_for_review", "review_requested"])
+# Retrieve the set of actions from the configuration
+pr_actions = set(get_settings().get("GITHUB_ACTION_CONFIG.PR_ACTIONS", ["opened", "reopened", "ready_for_review", "review_requested"]))
 
 if action in pr_actions:
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Using a set instead of a list can improve lookup performance, but the impact may be minimal for small lists.

    		6
    Best practice
    Use a more idiomatic Python approach for setting default values

    Consider using a default empty list and the or operator instead of get() with a
    default value for clearer and more idiomatic Python code.

    pr_agent/servers/github_action_runner.py [88]

    -pr_actions = get_settings().get("GITHUB_ACTION_CONFIG.PR_ACTIONS", ["opened", "reopened", "ready_for_review", "review_requested"])
+pr_actions = get_settings().get("GITHUB_ACTION_CONFIG.PR_ACTIONS") or ["opened", "reopened", "ready_for_review", "review_requested"]
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggested approach is more idiomatic Python, but the current implementation is also clear and functional.

    		5
    Enhancement
    Uncomment the configuration option to make it more visible and easily modifiable

    Consider uncommenting the pr_actions configuration to make it explicit and easier
    for users to modify.

    pr_agent/settings/configuration.toml [203]

    -# pr_actions = ['opened', 'reopened', 'ready_for_review', 'review_requested']
+pr_actions = ['opened', 'reopened', 'ready_for_review', 'review_requested']
    • Apply this suggestion
    Suggestion importance[1-10]: 3

    Why: Uncommenting the configuration might be helpful for visibility, but keeping it commented allows for clear documentation of default values without changing behavior.

    		3

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Sep 15, 2024

    @matanbaruch
    i am not against this change, but it needs to be accompanied by a documentation update to be merged

    https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-action
    https://github.com/Codium-ai/pr-agent/blob/main/docs/docs/usage-guide/automations_and_usage.md

    @matanbaruch
    Copy link
    Contributor Author

    @mrT23 Added

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Sep 15, 2024

    cool :-)

    @mrT23 mrT23 merged commit 80fe297 into qodo-ai:main Sep 15, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @mrT23 mrT23 mrT23 approved these changes

    Assignees
    No one assigned
    Labels
    enhancement New feature or request
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @matanbaruch @mrT23