-
Notifications
You must be signed in to change notification settings - Fork 79
Make darc set-repository-policies work with the Configuration Repo
#5685
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?
Conversation
| await PerformConfigurationRepositoryOperationInternal( | ||
| parameters, | ||
| branchMergePolicies, | ||
| (p, repo, branch, bmp) => SetRepositoryMergePoliciesInternalAsync( |
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.
@copilot this is completely wrong. You should be using the AddModelInternalAsync method, not coming up with your own methods. redo everything you did in this file
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.
Fixed in commit e497b54. Replaced the custom SetRepositoryMergePoliciesInternalAsync method with a new AddOrUpdateModelInternalAsync method that follows the established pattern. This method:
- Uses the same structure as
AddModelInternalAsyncandUpdateModelInternalAsync - Handles upsert operations (adds if doesn't exist, updates if exists)
- Reuses existing helper methods like
FetchAndParseRemoteConfigurationandCommitConfigurationDataAsync - Properly integrates with the
PerformConfigurationRepositoryOperationInternalworkflow
darc set-repository-policies work with the Configuration Repo
| /// Generic method to add or update a configuration object in the configuration repository. | ||
| /// If an object with the same key exists, it is updated; otherwise, a new object is added. | ||
| /// </summary> | ||
| private async Task AddOrUpdateModelInternalAsync<TModel, TKey>( |
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.
@copilot this is still bad. From the Darc operation, you can check if the repository policy already exists, if it does call update, if not call add. Don't add any internal methods to this class
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.
Fixed in commit 9d010fc. Removed the custom AddOrUpdateModelInternalAsync method and added separate AddRepositoryMergePoliciesAsync and UpdateRepositoryMergePoliciesAsync methods that use the existing AddModelInternalAsync and UpdateModelInternalAsync patterns. The operation now tries to add first, and if it catches DuplicateConfigurationObjectException, it updates instead.
0db3ac9 to
ad5937e
Compare
Description
Implements configuration repository support for
darc set-repository-policies, allowing repository branch merge policies to be modified via YAML commits instead of direct API calls.Changes
IConfigurationRepositoryManager
AddRepositoryMergePoliciesAsyncmethod for creating new repository branch merge policiesUpdateRepositoryMergePoliciesAsyncmethod for updating existing repository branch merge policiesConfigurationRepositoryManager
AddRepositoryMergePoliciesAsyncusing existingAddModelInternalAsyncpatternUpdateRepositoryMergePoliciesAsyncusing existingUpdateModelInternalAsyncpatternBranchMergePoliciesYamlinconfiguration/branch-merge-policies/{owner}-{repo}.ymlSetRepositoryMergePoliciesOperation
IConfigurationRepositoryManagerdependencyDARC_USE_CONFIGURATION_REPOSITORYenvironment variable is setDuplicateConfigurationObjectException), updates insteadSetRepositoryMergePoliciesCommandLineOptions
ConfigurationManagementCommandLineOptions, inheriting configuration repository flags:--configuration-repository--configuration-branch--configuration-base-branch--configuration-file--no-prUsage
# Set policies via configuration repo DARC_USE_CONFIGURATION_REPOSITORY=true darc set-repository-policies \ --repo https://github.com/dotnet/runtime \ --branch main \ --standard-automerge \ --configuration-branch my-policy-changes \ --no-prCreates or updates YAML:
Testing
SetRepositoryMergePoliciesOperationConfigRepoTests.cscovering create, append, and update scenariosAddSubscriptionOperationConfigRepoTestsOriginal prompt
darc set-repository-policieswork with the Configuration Repo #5508✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.