-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
populate-owners: resolve owner aliases #2572
populate-owners: resolve owner aliases #2572
Conversation
- Fix go vet complaining about wrong type in ErrorF - Print console output when there is an error - Configure username and email for test git repo - Ignore difference in commit ID between expected and actual orgRepo
Resolves aliases contained in the OWNERS files based on the contents of the OWNERS_ALIASES file. Also adds optional parameter to only update repositories which match a regex.
CC @wking |
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.
Seems like a reasonable approach
// in another string slice. Returns a new slice with the insert | ||
// slice replacing the elements between begin and end index. | ||
func insertStringSlice(insert []string, intoSlice []string, | ||
begin int, end int) []string { |
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.
When is end
used? To overwrite items?
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.
That's correct, it will overwrite anything between begin and end in the existing slice. In this case it should always just be the single element which is the alias name.
assertEqual(t, test.input, test.namespaced) | ||
} | ||
}) | ||
func TestResolveAliases(t *testing.T) { |
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.
Would love a test for the slice insert
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.
Sounds good, I'll add one!
Also update readme, add comments, and remove some console logging
766c570
to
a9f4c3a
Compare
@stevekuznetsov Does this look ok? I added tests for the slice insert and also updated the readme. |
@wking any thoughts here? |
I haven't had time to work through the diff. If it looks good to you, don't wait on me. |
Sounds good. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pgier, stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This changes the populate-owners tool to resolve OWNER_ALIASES into the individual OWNERS files. This avoids some complexity related to merging OWNER_ALIASES from different repositories, and also makes it easy to update OWNERS only for certain repositories.