- 
                Notifications
    You must be signed in to change notification settings 
- Fork 356
chore(CODEOWNERS): Use usernames instead of groups #10818
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
| @sschuberth GitHub complains that @bs-ondem does not have write access to the repository, should we just remove the entry for  | 
| Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@            Coverage Diff            @@
##               main   #10818   +/-   ##
=========================================
  Coverage     57.39%   57.39%           
  Complexity     1674     1674           
=========================================
  Files           346      346           
  Lines         12764    12764           
  Branches       1210     1210           
=========================================
  Hits           7326     7326           
  Misses         4972     4972           
  Partials        466      466           
 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
| /**/*Sw360* @oss-review-toolkit/kotlin-devs @bs-ondem @oheger-bosch | ||
| /advisor/ @oss-review-toolkit/kotlin-devs @MarcelBochtler @oheger-bosch | ||
| /clients/fossid-webapp/ @oss-review-toolkit/kotlin-devs @nnobelis | ||
| # Members of @oss-review-toolkit/kotlin-devs. | 
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.
I'd prefer to not quote any https://github.com/orgs/oss-review-toolkit/teams names here as I wanted to go through these with @oss-review-toolkit/tsc soon to clean up some redundancy and confusion there.
But we should probably document the general idea behind this "catch all" entry, like by saying "General owners of the Kotlin code base."
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.
How about first discussing how we want to organize this and then updating the descriptions in the file afterward, and keep the group based assignments until then? AFAIK we also have no public list of people who are committers (according to the role described in the charter) and if the "owners of legally relevant documents" proposed below are not equal to the TSC members this also needs to be documented somewhere.
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.
Of course I'm fine with discussing the GitHub team structure first. But even after that I'd not quote the (new) names here, just because it creates maintenance effort if teams change again.
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.
My point is that I want to describe why people are listed here, not explain what the file patterns mean. Currently this mostly maps to people who are in certain teams which also give them write permission to the repository. In future I would prefer something like "TSC members" or "Project committers" and also add links to the official lists of people with these roles so that it is clear how the lists are updated. For example, "Owners of legally relevant documents" does not explain why these users own these documents. That's why I wanted to keep the descriptions technical for now.
| # Members of @oss-review-toolkit/kotlin-devs. | ||
| * @fviernau @MarcelBochtler @mnonnenmacher @nnobelis @oheger-bosch @sschuberth | ||
|  | ||
| # Members of @oss-review-toolkit/core-devs. | 
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.
Also here, I'd prefer to more general comment to describe the idea, like "Contributors who usually review documentation."
| # Members of @oss-review-toolkit/core-devs. | ||
| /spdx-utils/src/main/ @fviernau @MarcelBochtler @mnonnenmacher @sschuberth @tsteenbe | ||
|  | ||
| # Members of @oss-review-toolkit/core-devs. | 
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.
The comment could be "Owners of legally relevant documents."
| 
 Yes, I think it's fine to not have a dedicated entry there, as that code has not really been touched in quite some time. | 
Use usernames instead of groups in the CODEOWNERS file to make the code owners more transparent to external contributors. This also solves the issue that when a team members picks up a review, it is indivudally assigned as a reviewer of the PR, and all other team members are removed from the reviewers list. While at it, remove some entries which have become redundant because new members were added to the kotlin-devs team in the meantime, and remove the entry for SW360 because the code has not been touched in a while. Signed-off-by: Martin Nonnenmacher <[email protected]>
06e3675    to
    7193599      
    Compare
  
    
Use usernames instead of groups in the CODEOWNERS file to make the code owners more transparent to external contributors.
This also solves the issue that when a team members picks up a review, it is indivudally assigned as a reviewer of the PR, and all other team members are removed from the reviewers list.
While at it, remove some entries which have become redundant because new members were added to the kotlin-devs team in the meantime.