asyncapi / .github

Location of all reusable community health files
29 stars 62 forks source link

We need another command for PRs which acts as re-request for review and also reminder for codeowners #211

Closed derberg closed 3 months ago

derberg commented 1 year ago

We have cases like https://github.com/asyncapi/spec/pull/847#pullrequestreview-1331087602 when PR owner has hard time:

So we have already commands like, ready-to-merge or autoupdate. We should have more:

quick proposals ☝🏼 that of course can be changed


before we talk about how to parse codeowners file, and then get data needed for requesting review, I suggest someone first check GitHub GraphQL or REST APIs as I think one day when I was exploring it, there was some API, probably GitHub Graphql mutation to trigger re-request review.

also maybe there are ready GitHub actions that we could use in workflows and there is no need to call API manually 🤔

arunavabasucom commented 1 year ago

@derberg can I work on this ??

derberg commented 1 year ago

@arunavabasu-03 that would be amazing, please go ahead. Lemme know if you have questions

Priyansh61 commented 1 year ago

Hi @derberg ,

I was exploring this feature and I just had certain doubts. Like for re-request we already have an option to call from the right-top

image

And for the ping-for-attention do we want a button which serves the same purpose as (@xyz)?

Thanks,

derberg commented 1 year ago

We do not talk here about buttons but commands, like we have help command -> https://github.com/asyncapi/.github/blob/master/.github/workflows/help-command.yml

ping-for-attention (can be a better name if you have one) is a commend that someone would write in PR: /ping-for-attention or /pfa and the workflow would:

in case of rerequest the feature you refer to sometimes behave strange -> https://github.com/asyncapi/spec/pull/847#issuecomment-1460495718. Contributor was using it but it was assigning and removing at the same time.

Screenshot 2023-05-17 at 15 12 03

anyway, not a problem that something can be done with a button, we use commands to not click buttons to often 😄

derberg commented 1 year ago

nice example of comment command used in practice -> https://github.com/asyncapi/website/issues/1684#issuecomment-1551521659

Priyansh61 commented 1 year ago

@derberg PTAL at the PR.

I had a doubt before proceeding for the PR of rerequest.

I went through the documentation of Github GraphQL and yes there is a mutation already present to request review https://docs.github.com/en/graphql/reference/mutations#requestreviews but we dont have any existing mutation for re-request review.

So when the user enters /rerequest. What should be the behaviour?

I am thinking that the user should provide from whom he wants the review (@) and then use `/rerequest', this way we can directly use the Github GtraphQL mutation present, and ask the specific reviewer.

Thanks

derberg commented 1 year ago

looks like review works like re-review too

I did

mutation { 
  requestReviews(input: {pullRequestId:"PR_kwDODyzcIc5Q0XEr", userIds:"U_kgDOAGq_1w"}) {
    clientMutationId,
    pullRequest {
      id
    }
    actor {
      login
    }
  } 
}

where U_kgDOAGq_1w is my user ID, and PR_kwDODyzcIc5Q0XEr is ID of this PR

after I fired this mutation, I noticed

Screenshot 2023-05-22 at 11 19 45

so /rereview should work this way:

Priyansh61 commented 1 year ago

query for IDs of current PR reviewers mutation to request review from these IDs. Looking at docs, userIds can be an array of IDs, so one mutation to request all

@derberg I just had a doubt like consider a scenario when a reviewer had already approved a PR, but some other reviewer has requested changes on it. So should we ask all the reviewers for a review?

Instead, we can have a better option that we can ask a particular person to review the PR again by having a comment @xyz @abc /review. This way we wont disturb other reviewers and can only ask from specific users.

We can have a small tweak here, that if there is no particular reviewer mentioned then we can ask all the requested reviewers to review again.

What's your opinion on this ?

github-actions[bot] commented 9 months ago

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

derberg commented 4 months ago

contributor do not respond so this issue is still available

many things done already in https://github.com/asyncapi/.github/pull/243 so you can continue the work that was started