Skip to content

Adding more CODEOWNERS #6422

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Adding more CODEOWNERS #6422

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa77157
Adding more CODEOWNERS
TGWDB Nov 1, 2021
d798a83
Remove @tautschnig from smt2 at their request
TGWDB Nov 1, 2021
deae483
Update with feedback from @tautschnig @thomasspriggs @NlightNFotis
TGWDB Nov 4, 2021
9d916e3
Remove contentious codeowner additions on sensitive code
TGWDB Nov 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
# approval within two weeks.
#
# These owners will be the default owners for everything in the repo.
* @kroening @tautschnig @peterschrammel @chrisr-diffblue
* @kroening @tautschnig @peterschrammel @chrisr-diffblue @TGWDB

# These files should rarely change

/src/big-int/ @kroening
/src/ansi-c/ @kroening @tautschnig @chrisr-diffblue
/src/assembler/ @kroening @tautschnig @chrisr-diffblue
/src/ansi-c/ @kroening @tautschnig @chrisr-diffblue @peterschrammel @thomasspriggs @NlightNFotis @TGWDB
/src/assembler/ @kroening @tautschnig @chrisr-diffblue @peterschrammel @thomasspriggs @NlightNFotis @TGWDB
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say that I am any kind of authority on our assembly support, having never worked on its implementation in cbmc. However I have enough familiarity with cbmc front-ends in general, that I am not going to strenuously object to being made a codeowner of this, if we need more codeowners.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dare I question whether the above changes are in line with "regularly updated parts of the code?"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assembler sub folder has not been updated particularly regularly, as I read the commit history. Therefore adding more code owners for it is not in line with this PR's "regularly updated parts of the code" description. @TGWDB What is your view on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know anything about what's in src/assembler myself, so I don't think I'm particularly suited for being a code owner for this.

Copy link
Contributor Author

@TGWDB TGWDB Nov 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll reduce some of these (commit to come shortly pushed). I agree they may not be "regular" as the PR suggests, on the other hand we had a problem with getting a PR approved by codeowner around the ansi-c part, so I'll add a little more here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it helps, my role has very recently changed so that I now have a lot more time to work on CBMC.

/src/goto-cc/ @kroening @tautschnig @chrisr-diffblue
/src/linking/ @kroening @tautschnig @chrisr-diffblue
/src/memory-models/ @kroening @tautschnig
Expand All @@ -30,9 +30,9 @@

# These files change frequently and changes are high-risk

/src/cbmc/ @kroening @tautschnig @peterschrammel
/src/goto-programs/ @kroening @tautschnig @peterschrammel
/src/util/ @kroening @tautschnig @peterschrammel
/src/cbmc/ @kroening @tautschnig @peterschrammel @thomasspriggs @NlightNFotis @TGWDB
/src/goto-programs/ @kroening @tautschnig @peterschrammel @thomasspriggs @NlightNFotis @TGWDB
/src/util/ @kroening @tautschnig @peterschrammel @thomasspriggs @NlightNFotis @TGWDB
/src/solvers/refinement @martin-cs @romainbrenguier @peterschrammel
/src/solvers/strings @martin-cs @romainbrenguier @peterschrammel
/jbmc/src/java_bytecode/ @romainbrenguier @peterschrammel
Expand All @@ -49,12 +49,12 @@
/doc/cprover-manual/contracts* @tautschnig @feliperodri @SaswatPadhi
/src/goto-diff/ @tautschnig @peterschrammel
/src/jsil/ @kroening @tautschnig
/src/memory-analyzer/ @tautschnig @chrisr-diffblue
/src/memory-analyzer/ @tautschnig @chrisr-diffblue @peterschrammel
/jbmc/src/jbmc/ @peterschrammel @romainbrenguier
/jbmc/src/janalyzer/ @peterschrammel @romainbrenguier
/jbmc/src/jdiff/ @peterschrammel
/jbmc/src/jdiff/ @peterschrammel @chrisr-diffblue
/src/cpp/ @kroening @tautschnig @peterschrammel
/src/solvers/smt2 @kroening @martin-cs @tautschnig @peterschrammel @allredj @romainbrenguier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While at it, please remove my name from this one. I do not feel sufficiently competent/confident for SMT2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in pending commit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK for me. I'm not active on SMT2 at the moment.

/src/solvers/smt2 @kroening @martin-cs @tautschnig @peterschrammel @thomasspriggs @NlightNFotis @TGWDB
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, I've worked on a bit in the past, however given that the aim is sunsetting it in favour of the new SMT backend, I'm not sure what value code-ownership of this brings (I expect this to be a low traffic area of the code, but then again I might be mistaken).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm tempted to keep this since we've had a few fixes and updates here, and with work on the new SMT side I can see some overlap in the future (bug fixes, minor refactors, etc.).

/src/solvers/smt2_incremental @peterschrammel @thomasspriggs @NlightNFotis @TGWDB @chrisr-diffblue
/src/statement-list/ @kroening @tautschnig @peterschrammel @pkesseli

Expand Down