🌱 Add owner files for docs/ and test/ #5469
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
size/M
enxebre
force-pushed
the
test-docs-areas
branch
5 times, most recently
from
38871d0 to
ac19676
Compare
enxebre
force-pushed
the
test-docs-areas
branch
from
ac19676 to
20d4b01
Compare
k8s-ci-robot
removed
the
do-not-merge/invalid-owners-file
|
/lgtm |
k8s-ci-robot
added
the
lgtm
enxebre
force-pushed
the
test-docs-areas
branch
from
20d4b01 to
abfae26
Compare
k8s-ci-robot
removed
the
lgtm
enxebre
force-pushed
the
test-docs-areas
branch
3 times, most recently
from
e0ecb80 to
0cc2bbe
Compare
enxebre
force-pushed
the
test-docs-areas
branch
from
0cc2bbe to
3b7d690
Compare
There was a problem hiding this comment.
@enxebre thanks for pushing forward this effort!
Only two nits from my side, but overall it looks great
enxebre
force-pushed
the
test-docs-areas
branch
from
3b7d690 to
025ba51
Compare
OWNERS_ALIASES
Outdated
| # ----------------------------------------------------------- | ||
| # OWNER_ALIASES for test | ||
| # ----------------------------------------------------------- | ||
|
|
||
| cluster-api-test-reviewers: | ||
| cluster-api-test-maintainers: |
There was a problem hiding this comment.
We should probably split test rather than have a unique one for all the subfolders, CAPD already has its own, maybe let's add one for the test framework?
There was a problem hiding this comment.
+1 for having owners specifically for the test framework to start with (as in the library)
There was a problem hiding this comment.
If we have one for test/infrastructure/docker and test/framework, should we have another one for test/e2e?
Note: we also have test/infrastructure/container, but it doesn't contain a lot of code.
There was a problem hiding this comment.
I'm ok with all the solutions, but I think that also the current solution make sense given that
- owners in
test/are owners of the test framework, E2E tests but also owners of CAPD that is used extensively in our E2E. - owners in
test/infrastructure/dockerare owner of CAPD only(we can eventually add test/infrastructure/container under the same responsibility now or at later stage)
There was a problem hiding this comment.
Ok I updated to include a new OWNERS file in test/framework so:
- Owners in test/ are owners of the test framework, E2E tests but also owners of CAPD that is used extensively in our E2E.
- Owners in test/infrastructure/docker are owner of CAPD only(we can eventually add test/infrastructure/container under the same responsibility now or at later stage)
- Owners in test/framework own the library.
I wouldn't go too far breaking this down until we have root owners in /test so they can figure out the best way for them to operate.
Let me know if that make sense or you want me to change something else.
enxebre
force-pushed
the
test-docs-areas
branch
from
025ba51 to
95afee9
Compare
k8s-ci-robot
added
size/L
enxebre
force-pushed
the
test-docs-areas
branch
from
95afee9 to
fa3aa14
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vincepri 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 |
k8s-ci-robot
added
the
approved
What this PR does / why we need it:
Add owner files for docs/ and test/
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Part of #5456