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.

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

Already on GitHub? Sign in to your account

feat: run block #827

Merged
merged 11 commits into from
Apr 14, 2023
Merged

feat: run block #827

merged 11 commits into from
Apr 14, 2023

Conversation

zolamk
Copy link
Contributor

@zolamk zolamk commented Apr 11, 2023

Description

Introduces the run field in a workflow which introduces a new way to define actions on a workflow, it's now possible to have actions without rules, actions with arbitrarily nested if, then, else conditions or a combination of both.

Related issue

Closes reviewpad/raas-common#102

Type of change

New feature (non-breaking change which adds functionality)

How was this tested?

Manual and unit tests(TBD)

Checklist

  • I have performed a self-review of my code
  • I have added tests that prove my fix/feature works (if applicable) and have tested properly
  • I have ran task check -f and have no issues

Code review and merge strategy (ship/show/ask)

Ask: this pull request requires a code review before merge

HOW

馃 Generated by Copilot at d0baa28

  • Add support for run blocks in workflows, which allow conditional execution of actions based on rules (link, link, link, link, link, link)
  • Modify the PadWorkflow type to include a field for the non-normalized run value and a field for the normalized runs slice, which contains PadWorkflowRunBlock structs (link)
  • Add a function to process the run field of the workflow and convert it to a slice of PadWorkflowRunBlock and a slice of PadRules, handling different types of values and parsing them according to the run block syntax (engine/inline_rules_normalizer.go, link)
  • Modify the processWorkflow function to also process the run field and append the run rules to the compact rules (engine/inline_rules_normalizer.go, link)
  • Add a loop to iterate over the workflow runs and append the actions from each run block to the program (engine/exec.go, link)
  • Add two helper functions to get the actions from a run block and a slice of run blocks, recursively evaluating the rules and the then/else branches (engine/exec.go, link)
  • Add the runs field to the transformed workflow struct (engine/loader.go, link)
  • Remove the check for the workflow having any rules, since it is now possible for a workflow to have a run block without any rules (engine/linter.go, link)
  • Add a case to handle nil values in the processCompactRules function, to avoid panicking when the workflow does not have any rules (engine/inline_rules_normalizer.go, link)

@reviewpad reviewpad bot added new-feature Non-breaking change which adds functionality ask Pull request requires a code review before merge large large pull request labels Apr 11, 2023
@reviewpad
Copy link

reviewpad bot commented Apr 11, 2023

AI-Generated Summary: This pull request contains 4 patches that introduce the following changes:

  1. Adds a new Run field to the PadWorkflow struct in lang.go, implementing the PadWorkflowRunBlock struct to support nested run blocks with if-then-else clauses.
  2. Changes the Run field to a slice of PadWorkflowRunBlock instances, allowing the run block to have multiple items.
  3. Removes the lint check for the number of rules in a workflow, as it is now possible for a workflow to have a run block without any rules by utilizing the newly added feature.
  4. Implements arbitrarily nested if-then-else structures in run blocks with updated parsing and evaluation functions.

Overall, these changes enable more complex and flexible workflow configurations in the engine.

@reviewpad
Copy link

reviewpad bot commented Apr 11, 2023

Reviewpad Report

鈥硷笍 Errors

  • The pull request does not have a linked issue. Please link one. Reviewpad will only assign a reviewer when the pull request is linked to an issue.

@reviewpad
Copy link

reviewpad bot commented Apr 12, 2023

AI-Generated Summary: This pull request includes 5 patches that implement and modify the run block feature in the language engine. The changes are as follows:

  1. Introduces the PadWorkflowRunBlock struct and adds a Run field to the PadWorkflow struct.
  2. Changes the Run field in PadWorkflow struct to Runs for allowing multiple run blocks, and updates the PadWorkflowRunBlock struct to have arrays for Then and Else.
  3. Removes the lint check for the number of rules in a workflow as it is now possible for a workflow to have a run block without any rules.
  4. Implements arbitrarily nested if-then-else in the run block by adding new functions to handle and process run blocks, such as getActionsFromRunBlock, getActionsFromRunBlocks and processRun.
  5. Adds a linter for run blocks to check if they have a 'then' block and no actions.

@reviewpad
Copy link

reviewpad bot commented Apr 12, 2023

AI-Generated Summary: This pull request contains updates in 6 patches. Changes include:

  1. Adding a Run field to PadWorkflow struct and a new PadWorkflowRunBlock struct in the engine/lang.go file.
  2. Updating the Run field in PadWorkflow struct to have multiple items, now named Runs.
  3. Refactoring the linter to remove the number of rules lint check in engine/linter.go since it's now possible to have a run block without rules.
  4. Implementing support for arbitrarily nested if-then-else blocks in the run section of workflows.
  5. Adding a linter to ensure every run block in a workflow has a then block or actions.
  6. Adding test coverage for different cases of run block in engine/exec_test.go.

These updates improve the functionality and flexibility of the existing code structure, allowing for more complex workflows with conditionally nested actions.

@zolamk zolamk marked this pull request as ready for review April 12, 2023 07:33
@reviewpad
Copy link

reviewpad bot commented Apr 12, 2023

AI-Generated Summary: This pull request contains the following 7 patches which add, modify and test the run block feature in the engine:

  1. Add run block to the PadWorkflow struct in engine/lang.go
  2. Change run block to have multiple items in engine/lang.go
  3. Remove number of rules linting in engine/linter.go
  4. Implement arbitrarily nested if-then-else logic in run block for both engine/exec.go and engine/inline_rules_normalizer.go
  5. Add linting for run blocks in engine/linter.go
  6. Add test coverage for different run block scenarios in engine/exec_test.go
  7. Add comments to explain the different run block types in engine/inline_rules_normalizer.go.

The changes include support for single actions, lists of actions, and nested if-then-else logic in the run block.

@reviewpad reviewpad bot added the waiting-review PR waiting for review label Apr 12, 2023
@reviewpad
Copy link

reviewpad bot commented Apr 12, 2023

AI-Generated Summary: This pull request introduces various updates in the engine, specifically related to the PadWorkflow and the handling of run blocks. The following are the main changes included:

  1. Added a Run field in PadWorkflow to store the run block and created a new PadWorkflowRunBlock type to handle the run block's if-then-else structure.
  2. Changed Run to Runs in PadWorkflow to support multiple items and updated its type to []PadWorkflowRunBlock.
  3. Removed the number of rules lint check, as it's now possible for a workflow to have a run block without any rules.
  4. Implemented nested if-then-else logic in run blocks, allowing arbitrary nesting of conditions within run blocks.
  5. Extended linting to cover run blocks and ensure that each block has a then block and/or an action.
  6. Introduced test coverage to support new run block features (scalar run blocks, list of run blocks, and combination of run blocks).

By including these changes, the engine now supports more complex run block structures, allowing users to create workflows with more advanced logic and conditions.

@reviewpad
Copy link

reviewpad bot commented Apr 13, 2023

AI-Generated Summary: This pull request introduces support for flexible run blocks in code review workflows. The changes include new test cases and modifications to the following files:

  • engine/exec.go: Added support for evaluating actions from run blocks and updated EvalConfigurationFile function.
  • engine/exec_test.go: Added test cases for different types of run blocks.
  • engine/inline_rules_normalizer.go: Implemented support for normalizing run blocks.
  • engine/lang.go: Added a new struct PadWorkflowRunBlock to represent run blocks.
  • engine/linter.go: Updated linting functions to handle run blocks without rules.
  • engine/loader.go: Added support for transforming workflows with run blocks.
  • Added new test YAML files in engine/testdata/exec/ to cover different types of run blocks.

Overall, these changes allow users to better customize their workflows in code review processes.

Base automatically changed from feat/compact-if-then-action to main April 13, 2023 08:43
@reviewpad
Copy link

reviewpad bot commented Apr 13, 2023

AI-Generated Summary: This pull request introduces changes that enhance Reviewpad's configuration handling by supporting different structures for the run block in workflows. It adds support for simple strings, lists of actions, single conditionals, single conditionals with else, and combinations of these structures. The new logic allows for more flexibility when defining workflows and their actions in configuration files. Test cases have been added to verify the correct behavior of these new run block structures, and existing components have been updated accordingly. Overall, this update enhances the capabilities of Reviewpad and improves the user experience when working with configuration files.

Copy link
Member

@ferreiratiago ferreiratiago left a comment

Choose a reason for hiding this comment

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

Overall, this is great. I have some mix feelings about the way engine/inline_rules_normalizer.go is being organized. It's becoming big and hard to maintain. I'll iterate on this and get back to you with more feedback.

engine/testdata/exec/reviewpad_with_combination_run.yml Outdated Show resolved Hide resolved
engine/testdata/exec/reviewpad_with_string_run.yml Outdated Show resolved Hide resolved
engine/linter.go Outdated Show resolved Hide resolved
engine/inline_rules_normalizer.go Outdated Show resolved Hide resolved
@reviewpad reviewpad bot added requires-author-attention Pull request requires the author to take action and removed waiting-review PR waiting for review labels Apr 13, 2023
@reviewpad
Copy link

reviewpad bot commented Apr 13, 2023

AI-Generated Summary: This pull request introduces several changes to the engine to support different types of workflow execution. It adds the ability to have a list of actions or conditional blocks in the run section of a workflow. The code now supports single actions, lists of actions, single conditionals, single conditionals with else, and combinations of these elements in a single run block. The changes include modifications to the EvalConfigurationFile, getActionsFromRunBlock, and getActionsFromRunBlocks functions in engine/exec.go, as well as updates to the inline_rules_normalizer.go, lang.go, linter.go, loader.go, and relevant test files.

@reviewpad reviewpad bot added the waiting-review PR waiting for review label Apr 13, 2023
@reviewpad reviewpad bot removed the requires-author-attention Pull request requires the author to take action label Apr 13, 2023
@reviewpad
Copy link

reviewpad bot commented Apr 13, 2023

AI-Generated Summary: This pull request introduces several enhancements to the Reviewpad engine. Mainly, it adds support for the new run block structure in workflows, allowing for more complex conditional logic and nested conditions. The changes include:

  1. Implementing getActionsFromRunBlock, getActionsFromRunBlocks, and related functions in exec.go to process and evaluate the run blocks in workflows.
  2. Adding test cases for the new run block processing in exec_test.go.
  3. Implementing normalizeRun function in inline_rules_normalizer.go to normalize run blocks in configuration files.
  4. Modifying the PadWorkflow struct in lang.go to support the new run block structure with separate Runs, If, Then, Else, and Actions fields.
  5. Updating the linter in linter.go to validate the new run block structures in workflows.
  6. Updating the Reviewpad file transformation in loader.go to include the new run block fields in workflows.
  7. Adding new test configuration files for testing the different run block scenarios.

This work adds richer control flow mechanisms to the code review automation platform and improves overall functionality.

@reviewpad
Copy link

reviewpad bot commented Apr 13, 2023

AI-Generated Summary: This pull request introduces support for run blocks in workflows, allowing more complex configurations and conditional execution of actions. It adds new test cases for different types of run blocks such as single-action, list of actions, single conditional, single conditional with else, and a combination of these. It also validates that a run block has a then block or actions, and it updates the workflow transformation and linting logic to accommodate the new run blocks. Additionally, several new test files with different run block configurations are added.

@ferreiratiago ferreiratiago merged commit 67e6f64 into main Apr 14, 2023
@ferreiratiago ferreiratiago deleted the feat/else-if branch April 14, 2023 11:27
@reviewpad reviewpad bot removed the waiting-review PR waiting for review label Apr 14, 2023
@reviewpad
Copy link

reviewpad bot commented Apr 14, 2023

馃搱 Pull Request Metrics

馃捇 Coding Time: 18 hours
馃浕 Pickup Time: 1 day
馃憖 Review Time: 1 day

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ask Pull request requires a code review before merge large large pull request new-feature Non-breaking change which adds functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants