Skip to content

ci: implementing migrator workflow #3878

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open

Conversation

germa89
Copy link
Collaborator

@germa89 germa89 commented Apr 30, 2025

Summary

As the title.

This pull request introduces a new GitHub Actions workflow, migrator.yml, designed to handle pull requests originating from forks. The workflow ensures that secrets required for CI processes can be accessed by migrating the forked PR into a branch within the base repository.

Changes

  • New Workflow: Added migrator.yml to workflows.

  • Trigger Events:

    • Automatically triggered by comments containing @pyansys-ci-bot migrate or @pyansys-ci-bot sync on pull requests.
    • Can also be triggered manually via workflow_dispatch with an issue number input.
  • Key Steps:

    • Reacts to comments to confirm or denies the action
    • Fetches details about the source branch and repository.
    • Clones the PyMAPDL repository and add the forked repo as remote.
    • Creates a new branch in the base repository, merges the forked branch, and pushes the changes.
    • Opens a new pull request in the base repository if one does not already exist.
    • Posts comments to indicate success or failure of the migration process.

This workflow simplifies the process of handling forked pull requests, improving developer experience.

Example of fork PR: #3868
Example of migrated PR: #3886

Summary by Sourcery

Implement a GitHub Actions workflow to migrate pull requests from forks, enabling secure CI processes and improving developer experience

New Features:

  • Developed automated PR migration process that creates a new branch in the base repository
  • Added comment reactions and status updates for migration workflow

CI:

  • Added migrator workflow to handle pull requests from forks
  • Implemented workflow triggered by comments or manual dispatch
  • Added member verification before migration process

Chores:

  • Created changelog entry for the migrator workflow implementation

germa89 added 23 commits April 24, 2025 11:26
…ling with improved branch checks and comments
@germa89 germa89 requested a review from a team as a code owner April 30, 2025 11:04
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@germa89 germa89 changed the title chore: hello ci: implementing migrator workflow Apr 30, 2025
@germa89 germa89 self-assigned this Apr 30, 2025
@germa89 germa89 requested a review from RobPasMue April 30, 2025 20:30
@github-actions github-actions bot added CI/CD Related with CICD, Github Actions, etc maintenance General maintenance of the repo (libraries, cicd, etc) labels Apr 30, 2025
@germa89 germa89 requested a review from klmcadams April 30, 2025 20:32
@germa89
Copy link
Collaborator Author

germa89 commented Apr 30, 2025

I think this is more or less ready. The conflict in the changes is still not clear to me, so I would appreciate any suggestions on this.

Pinging @RobPasMue since I discussed this topic with him and pinging @klmcadams because i would love her feedback on the conflict resolution.

@klmcadams
Copy link

Hi @germa89, I've resolved conflicts with cherry-picks like this, but I found this git-mergetool command that could be helpful for you if you're seeing the conflicts when doing the git merge. Here's a stackoverflow thread about how to use git-mergetool.

Copy link
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

Overall looks good =)

@germa89
Copy link
Collaborator Author

germa89 commented May 7, 2025

@klmcadams @RobPasMue can you check the conflict logic in 03bfc29

Additionally, since this is almost ready, pinging @ansys/pyansys-core for awareness, since this might become part of ansys/actions.

@germa89
Copy link
Collaborator Author

germa89 commented May 7, 2025

@sourcery-ai review

Copy link

sourcery-ai bot commented May 7, 2025

Reviewer's Guide

This pull request implements a new GitHub Actions workflow, migrator.yml, to automate the migration of pull requests from forks into the base repository. This is achieved by first verifying the triggering user's team membership. If authorized, the workflow fetches the forked branch, creates a new branch in the base repository, and merges the changes, including a strategy for automated merge conflict resolution that prefers changes from the fork. Finally, it uses the gh CLI to create or update a pull request in the base repository from this new branch and posts status comments (success or failure) on the original PR.

Sequence Diagram for Successful PR Migration by migrator.yml

sequenceDiagram
    actor User
    participant GitHub
    participant MigratorWorkflow as "migrator.yml Workflow"
    participant BaseRepo as "ansys/pymapdl (Base Repo)"
    participant ForkRepo as "Forked Repository"

    User->>GitHub: Comments "@pyansys-ci-bot migrate" on Fork PR
    GitHub->>MigratorWorkflow: Triggers workflow (on issue_comment)
    MigratorWorkflow->>MigratorWorkflow: Determine PR number and triggering user
    MigratorWorkflow->>GitHub: API Call: Check user team membership (ansys/pymapdl-maintainers)
    GitHub-->>MigratorWorkflow: User is a member
    MigratorWorkflow->>GitHub: API Call: React "+1" to comment
    MigratorWorkflow->>GitHub: API Call: Get Fork PR details (head branch, repo)
    GitHub-->>MigratorWorkflow: Fork PR details
    MigratorWorkflow->>BaseRepo: Git: Checkout base repository
    MigratorWorkflow->>ForkRepo: Git: Add fork as remote, Fetch/Pull PR branch
    MigratorWorkflow->>BaseRepo: Git: Create new branch (e.g., migration3/pr-XXX)
    MigratorWorkflow->>BaseRepo: Git: Merge forked branch into new branch (resolve conflicts if any)
    MigratorWorkflow->>BaseRepo: Git: Push new branch to BaseRepo
    MigratorWorkflow->>GitHub: API Call: Check if migrated PR already exists for the new branch
    GitHub-->>MigratorWorkflow: No existing migrated PR / or PR needs update
    MigratorWorkflow->>GitHub: API Call: Create new PR in BaseRepo (if not exists)
    GitHub-->>MigratorWorkflow: New PR created/identified (e.g., #YYY)
    MigratorWorkflow->>GitHub: API Call: React "rocket" to original comment
    MigratorWorkflow->>GitHub: API Call: Post success comment on original Fork PR (links to #YYY, if newly created)
Loading

Sequence Diagram for Unauthorized PR Migration Attempt

sequenceDiagram
    actor User
    participant GitHub
    participant MigratorWorkflow as "migrator.yml Workflow"

    User->>GitHub: Comments "@pyansys-ci-bot migrate" on Fork PR
    GitHub->>MigratorWorkflow: Triggers workflow (on issue_comment)
    MigratorWorkflow->>MigratorWorkflow: Determine PR number and triggering user
    MigratorWorkflow->>GitHub: API Call: Check user team membership (ansys/pymapdl-maintainers)
    GitHub-->>MigratorWorkflow: User is NOT a member
    MigratorWorkflow->>GitHub: API Call: React "-1" to comment
    Note over MigratorWorkflow: Workflow terminates further migration steps for non-members.
Loading

File-Level Changes

Change Details Files
Implemented a new GitHub Actions workflow (migrator.yml) to automate migrating fork PRs to the base repository, enabling CI processes requiring secrets.
  • Configured workflow to trigger on specific comments (@pyansys-ci-bot migrate or @pyansys-ci-bot sync) or manual workflow_dispatch.
  • Added contents: write and pull-requests: read permissions necessary for its operations.
  • Replaced a hardcoded user check with a dynamic team membership verification (pymapdl-maintainers) for authorizing workflow execution.
  • Integrated comment reactions (+1 for authorized, -1 for unauthorized) to provide immediate feedback on trigger comments.
/.github/workflows/migrator.yml
Developed the core logic for fetching, merging (with automated conflict resolution), and pushing forked PR changes into a new base repository branch, and managing the associated PR.
  • Fetches the source branch from the fork and creates/updates a corresponding branch (e.g., migration3/pr-NUMBER) in the base repository.
  • Implemented automated merge conflict resolution by preferring incoming changes from the forked branch (using git checkout --theirs .).
  • Utilizes the gh CLI to check for an existing PR or create a new one in the base repository from the migrated branch.
  • Ensures the created/updated PR in the base repository links to and is set to close the original PR from the fork.
/.github/workflows/migrator.yml
Enhanced communication by adding steps to post status updates as comments on the original pull request.
  • Posts a success comment to the original PR, including a link to the newly created/updated PR in the base repository, upon successful migration.
  • Posts an error comment to the original PR if the migration process fails, prompting users to check action logs.
/.github/workflows/migrator.yml
Added a changelog entry to document the introduction of the new migrator CI workflow.
  • Created a new changelog file to record the 'ci: implementing migrator workflow' change.
/doc/changelog.d/3878.maintenance.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @germa89 - I've reviewed your changes - here's some feedback:

  • The git operations for syncing an existing migrated branch may not function as intended due to git checkout -b and an earlier git pull targeting an unexpected branch.
  • Confirm that automatically resolving merge conflicts by taking the fork's changes (--theirs) is the desired behavior when syncing, as it will overwrite any direct modifications to the migrated branch in the base repository.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +123 to +132
echo "Resolving conflicts by taking 'theirs' changes"
git checkout --theirs .
git add .

# Verify if conflicts are resolved
REMAINING_CONFLICTS=$(git ls-files -u | wc -l)
if [ "$REMAINING_CONFLICTS" -gt 0 ]; then
echo "Error: Conflicts remain after resolution. Aborting."
exit 1
fi
Copy link

Choose a reason for hiding this comment

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

suggestion (bug_risk): Review automatic conflict resolution strategy.

Confirm that automatically using 'git checkout --theirs' won’t discard crucial PR changes.

Suggested implementation:

            # Resolve conflicts by taking "theirs" changes if AUTO_RESOLVE flag is set.
            if [ "$AUTO_RESOLVE" == "true" ]; then
              echo "Auto-resolving conflicts using 'theirs' strategy"
              git checkout --theirs .
              git add .
            else
              echo "Automatic conflict resolution is disabled. Please resolve conflicts manually."
              exit 1
            fi

Ensure that the environment variable AUTO_RESOLVE is defined in your workflow configuration or the environment. This change guarantees that using the "theirs" strategy does not inadvertently discard crucial PR changes without explicit confirmation.

@klmcadams
Copy link

@germa89 The bot makes a good point about accidentally overwriting files. Is PR_HEAD_BRANCH typically main and PR_BASE_BRANCH the migrator branch? If so, when you're merging main into the migrator branch and there's a conflict, it could make sense to do git checkout --ours . instead of theirs to keep the changes from the migrator branch? Or manually check the conflicts like the bot recommended

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Related with CICD, Github Actions, etc maintenance General maintenance of the repo (libraries, cicd, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants