-
Notifications
You must be signed in to change notification settings - Fork 119
Add support for cleaning status checks during migration #1419
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
base: main
Are you sure you want to change the base?
Add support for cleaning status checks during migration #1419
Conversation
- Introduced `--clean-status-checks` option in the migrate command to clean status checks from GitHub branch protection after migration. - Implemented logic in `MigrateRepoCommandHandler` to handle the cleaning of status checks. - Added unit tests to verify the behavior of the new option and its impact on branch protection.
There was a problem hiding this 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 adds support for automatically cleaning status checks from GitHub branch protection rules after migration from Azure DevOps. The feature addresses the common issue where ADO status checks that are incompatible with GitHub get transferred during migration and require manual cleanup.
- Adds a new
--clean-status-checks
command-line option to themigrate-repo
command - Implements post-migration cleanup that removes status checks from the default branch while preserving other protection settings
- Includes comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs |
Implements the core logic for cleaning status checks from branch protection after migration completion |
src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs |
Adds the CleanStatusChecks property to command arguments |
src/ado2gh/Commands/MigrateRepo/MigrateRepoCommand.cs |
Adds the --clean-status-checks command-line option with description |
src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandTests.cs |
Adds tests for the new command option and argument handling |
src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs |
Comprehensive tests for the status check cleaning functionality |
src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs |
Tests for new GitHub API methods for branch protection management |
src/Octoshift/Services/GithubApi.cs |
Adds GitHub API methods for getting and updating branch protection rules |
README.md |
Documents the new feature with usage examples and explanations |
Unit Test Results 1 files 1 suites 10m 24s ⏱️ Results for commit 3b4e544. ♻️ This comment has been updated with latest results. |
…eline trigger management
@boylejj can you please assign a team for this review? |
My impulse is to say that this kind of change is best implemented as a step in the migration itself. Is there a reason to put it in the CLI, other than "it's the part that's handy"? |
It's an optional argument that we'd like to leverage migrating 1000s repos at the same time. Having enriched and flexible CLI features eliminates a need to script additional logic. |
@begonaguereca this can be reviewed independently as well |
I tend to agree with this being more backend logic. But in the meantime, are there screenshots of a functional test with this logic working please to make it clear what this does? |
@georgy-gorelko and what is the before and after of running with this flag, want to understand what settings were migrated incorrectly and what we are cleaning up. |
@georgy-gorelko also thoughts on using something like this https://github.com/katiem0/gh-branch-rules which should solve your problem and then would allow us to fix this properly on the backend (as it seemingly is a bug). Also sorry for multiple questions when you stated |
Require status check for above (shown in screenshot) blocks all PRs. ADO status checks for custom checks not supported in GitHub (Example: ADO code coverage, Component Governance, etc). Github has its own version of these checks implemented and supported as such ADO transition is unnecessary and requires manual cleanup/disablement upon migration. |
Upon migration I have found that ADO status checks hang on waiting status. My understanding is that these checks from ADO have no report back to GitHub and simply need to be regenerated by GitHub rulesets. https://github.com/katiem0/gh-branch-rules lists/updates GH repo policies but doesn't correlate ADO status checks being transferred during migration process. Thus cleaning upon ADO to GH migration required. |
Any concerns on merging this PR? |
@georgy-gorelko I started discussion with team as this seems like something better handled on backend. From my understanding, we don't need to migrate these status checks for ADO, and if that is the case we should handle that in our core migration logic. |
@georgy-gorelko I'm working on a backend change that will not migrate these status checks for ADO, as they are seemingly not compatible with GitHub. |
@georgy-gorelko do you have a test org you are using for testing this migration behavior? |
@georgy-gorelko as mentioned I will be out of office soon so whoever has bandwidth from the team will pick up my PR I created on the backend to achieve this functionality. We will just need your test org name for you to test once shipped. |
I do testing in our prototype project https://msdyneng.visualstudio.com/RuneDevOps. Not sure if you have an access to it. I can assist with testing though |
We are merging in https://github.ghe.com/github/octoshift/pull/11209 which takes care of handling this logic in the GEI backend 👍🏽 |
Closing PR given above PR was raised. Will reopen if issue still persists |
Reopening given status checks still blocking PRs after the migration |
@brianaj @begonaguereca status checks still transfer during the migration that require manual undo. Can you please confirm that backend changes committed and verified? Otherwise let me know if I can proceed with current PR |
Cleaning Status Checks during Migration
When migrating from Azure DevOps, both branch policies and status checks are transferred to GitHub. However, many ADO status checks are not supported in GitHub and require manual cleanup. To clean status checks from GitHub branch protection rules after migration automatically, use the
--clean-status-checks
option:This option will:
Note: This is a post-migration cleanup step, so it only affects the final GitHub repository state and does not impact the migration data transfer itself.