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

Fix null requested_reviewer from API #31773

Conversation

emrebdr
Copy link
Contributor

@emrebdr emrebdr commented Aug 3, 2024

If the assign the pull request review to a team, it did not show the members of the team in the "requested_reviewers" field, so the field was null. As a solution, I added the team members to the array.

fix #31764

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 3, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 3, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Aug 3, 2024
@emrebdr emrebdr changed the title fix null pull request reviewers from api if reviews assigned to team Fix null requested_reviewer from API Aug 3, 2024
@kemzeb
Copy link
Contributor

kemzeb commented Aug 4, 2024

Thanks for the contribution!

I think it maybe better to create a separate array field in the DTO object for requested teams (similar to GitHub, see here).

To me this makes more sense performance-wise (e.g. there can be many members in a team and, if I'm not mistaken, there may be multiple teams reviewing a PR; this is maybe too much data for us to send that consumers may not care much about).

I guess it makes sense logically too since we aren't requesting a review from individual team members but rather the team as a whole. This shouldn't be a breaking change either.

@emrebdr
Copy link
Contributor Author

emrebdr commented Aug 4, 2024

Thanks for the contribution!

I think it maybe better to create a separate array field in the DTO object for requested teams (similar to GitHub, see here).

To me this makes more sense performance-wise (e.g. there can be many members in a team and, if I'm not mistaken, there may be multiple teams reviewing a PR; this is maybe too much data for us to send that consumers may not care much about).

I guess it makes sense logically too since we aren't requesting a review from individual team members but rather the team as a whole. This shouldn't be a breaking change either.

so you mean create a separate array in response object for teams right? And this array field returns teams information? 🤔

@kemzeb
Copy link
Contributor

kemzeb commented Aug 4, 2024

so you mean create a separate array in response object for teams right? And this array field returns teams information? 🤔

Yes

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 4, 2024
@github-actions github-actions bot added the modifies/api This PR adds API routes or modifies them label Aug 4, 2024
@emrebdr
Copy link
Contributor Author

emrebdr commented Aug 4, 2024

then I think now better.

Ready for review

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 5, 2024
@lunny lunny added this to the 1.23.0 milestone Aug 5, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 5, 2024
@wolfogre wolfogre added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 5, 2024
@wolfogre wolfogre enabled auto-merge (squash) August 5, 2024 10:32
@wolfogre wolfogre merged commit 94cca88 into go-gitea:main Aug 5, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 5, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 7, 2024
* giteaofficial/main:
  Fix protected branch files detection on pre_receive hook (go-gitea#31778)
  Add signature support for the RPM module (go-gitea#27069)
  Fix null requested_reviewer from API (go-gitea#31773)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Details about pull request reviewers that are groups are not returned from the API
5 participants