Skip to content
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

chore: Automated request for reviews of pull request #50991

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 24, 2025

Ensure that every PR we receive gets the correct reviewers assigned.

@provokateurin
Copy link
Member

I think we should also assign some people, when neither backend nor frontend applies. Otherwise we still have PRs that would slip through.

@susnux
Copy link
Contributor Author

susnux commented Feb 24, 2025

I think we should also assign some people, when neither backend nor frontend applies. Otherwise we still have PRs that would slip through.

I was thinking the same, but who to assign? Basically what is currently not handled:

  • package.json -> dependabot spam -> already handled by our dependabot assign action
  • workflows
  • documentation

@provokateurin
Copy link
Member

My concern was mainly a bunch of the scripts that are used for building, testing or other purposes.

@susnux
Copy link
Contributor Author

susnux commented Feb 25, 2025

My concern was mainly a bunch of the scripts that are used for building, testing or other purposes.

Yes but who would be a good assignment for build/? Its not directly backend and according to GitHub we should always request @nickvergessen for review, but I am pretty sure he would be not happy about it.

(the other PR also does not include any code owner for build scripts anyway).

@nickvergessen
Copy link
Member

nickvergessen commented Feb 25, 2025

Yes but who would be a good assignment for build/? Its not directly backend

build/integration/, build/psalm/ and build/stubs/ are server-backend

The files in build itself are mostly CI or security, I'm happy to help/assist with those (apart from build/psalm-baseline*.xml which is server-backend again)


Is it possible to check if there are assignees already? If a knowing engineer pinged their own team or related engineers on an API change (e.g. groupware on CalDAV), there is no need to flood and waste valuable time of server backenders

Edit: Ah, this should be handled by if: github.event.pull_request.requested_reviewers[1] == null already.

@skjnldsv
Copy link
Member

Can we merge and try? :)

@susnux susnux marked this pull request as ready for review February 26, 2025 09:02
@susnux
Copy link
Contributor Author

susnux commented Feb 27, 2025

Can we merge and try? :)

Just needs another review, we can drop or adjust anytime :)

Ensure that every PR we receive gets the correct reviewers assigned.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@nickvergessen nickvergessen merged commit 81073a7 into master Feb 28, 2025
188 of 193 checks passed
@nickvergessen nickvergessen deleted the chore/request-reviews branch February 28, 2025 08:11
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
'/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers' \
-f "team_reviewers[]=nextcloud/server-backend"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its not scoped 🤦 should be:

-f "team_reviewers[]=server-backend"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants