Don't use cbmc-developers as a code owner #2589
Conversation
johnnonweiler
requested review from
chrisr-diffblue,
forejtv,
kroening,
peterschrammel,
smowton,
tautschnig and
thk123
as code owners
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: 47a1eec).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/79598239
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: 8a46ebe).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/79612636
|
@peterschrammel Thanks for adding your commit to "Distribute former default codeowner entries". |
|
In general I feel OK-ish with this change, but I note that one subtle change of behaviour with this proposal is how newly created directories might be handled. In the old CODEOWNERS scheme with 'default owners', any newly created directory would, at the very least, fall under the default owners unless otherwise explicitly listed in CODEOWNERS. In this new scheme, whether or not a new directory gets code owners will depend on exactly where it is created... e.g. a hypothetical new directory |
| @@ -20,7 +20,7 @@ const char *irep_ids_table[]= | |||
| #define IREP_ID_ONE(id) #id, | |||
| #define IREP_ID_TWO(id, str) #str, | |||
|
|
|||
| #include "irep_ids.def" | |||
| #include "ids/irep_ids.def" | |||
There was a problem hiding this comment.
Why is a PR that's supposedly only about the codeowners file changing these?
There was a problem hiding this comment.
(As the description above states, "In order to remove the code owner from irep_ids.def, this file was moved into its own subfolder of src/utils.") We want to give an owner to all the other files in src/utils, and we can't override that rule to remove the owner from a single file. However, it is possible to set an owner for all files in src/utils but not its subfolders. We moved irep_ids.def into a separate folder so that it can have its own (empty) list of owners.
There was a problem hiding this comment.
Happy with this once @peterschrammel has answered the question above.
|
We have come to the conclusion that Owen's original suggestion of using empty lists of owners is a better solution than this one. (It uses undocumented functionality, but it worked fine when we tried it in another repository.) See #2601. |
This is a suggested replacement for #2580. It changes the code owners file so that there are no default owners, and then removes the owners from the low-risk files which previously had diffblue/cbmc-developers as owner.
In order to remove the code owner from
irep_ids.def, this file was moved into its own subfolder of src/utils.@owen-jones-diffblue previously suggested an alternative approach to achieving the same result as this PR. He suggested giving some folders an empty list of owners. GitHub's (very limited) documentation on CODEOWNERS mentions 'one or more' owners, but the approach could have advantages if it did work.
This PR should be followed up by another PR to tidy up the CODEOWNERS file. This PR leaves a comment, that "These folders were previously covered by the default owners", but it would be better to reassess who the owner should be on these folders, and then tidy up the file with more helpful comments.