Skip to content

Remove ownership of the ocaml/ directory#1985

Merged
goldfirere merged 4 commits intomainfrom
rae/ocaml-is-unowned
Nov 8, 2023
Merged

Remove ownership of the ocaml/ directory#1985
goldfirere merged 4 commits intomainfrom
rae/ocaml-is-unowned

Conversation

@goldfirere
Copy link
Copy Markdown
Collaborator

@goldfirere goldfirere commented Nov 3, 2023

As title.

We seem to have converged on the idea of killing off CODEOWNERS, but I wasn't sure quite how far to take it. Please add more changes to this branch!

@ncik-roberts
More drastic removal (see commit for list):
63cd57a 
  * ocaml/
  * native_toplevel/
  * driver/
  * file_formats/
  * tools/
  * scripts/
  * specific tests
  * top-level metadata files
@ncik-roberts
Copy link
Copy Markdown
Contributor

ncik-roberts commented Nov 3, 2023

I propose a more drastic removal (done in my latest commit). All that's left is:

  • chamelon/
  • middle_end/
  • backend/
  • external/memtrace/
  • Some misc top-level files: codeowners, an opam file, build_ocaml_compiler.sexp

I leave the ones where I think there's arguably a larger benefit to automatically blocking review on people / automatically notifying them when the files change.

Anything we should add back in?

@goldfirere goldfirere mentioned this pull request Nov 7, 2023
Merged
lpw25
lpw25 approved these changes Nov 7, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@lpw25 lpw25 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm in favour of this change, but I wonder if it should be even more aggresive: @gretay-js and @mshinwell do you still want CODEOWNERS on for all of backend and middle_end, or should we remove those entries too?

@mshinwell
Copy link
Copy Markdown
Collaborator

mshinwell commented Nov 7, 2023

I think I'm happy to remove it from middle_end/ at least for the moment.

@goldfirere
Copy link
Copy Markdown
Collaborator Author

goldfirere commented Nov 7, 2023

I've removed ownership from middle_end as per Mark's suggestion. Waiting for a signal from @gretay-js before pulling the trigger here.

@gretay-js
Copy link
Copy Markdown
Contributor

gretay-js commented Nov 7, 2023 •
edited
Loading

I don't think that we need explicit code ownership in backend.

@goldfirere goldfirere merged commit 19fce80 into main Nov 8, 2023
@goldfirere goldfirere deleted the rae/ocaml-is-unowned branch November 8, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpw25 lpw25 lpw25 approved these changes

@mshinwell mshinwell Awaiting requested review from mshinwell mshinwell is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@goldfirere @ncik-roberts @mshinwell @gretay-js @lpw25