Skip to content

Conversation

@DavidSpickett
Copy link
Collaborator

This makes it more clear what you the author must do, and what reviewers can expect you to do, before an approved PR can be merged. Spliting out the email bit into a section also means we can link directly to it in discussions.

This relies on one of those parties actually reading this, but I plan to tackle the case where they don't with some new automation.

This makes it more clear what you the author must do, and what
reviewers can expect you to do, before an approved PR can be merged.

Spliting out the email bit into a section also means we can link
directly to it in discussions.

This relies on one of those parties actually reading this, but
I plan to tackle the case where they don't with new automation,
coming later.
Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

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

LGTM w/optional suggestion.

Copy link
Member

@rengolin rengolin left a comment

Choose a reason for hiding this comment

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

LGTM for what it's worth. I think the label will help.

Copy link
Contributor

@banach-space banach-space left a comment

Choose a reason for hiding this comment

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

Thanks David!

Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

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

BTW, I am not sure if it is possible to make a bot that suggests this when:

  • The pr is approved. And,
  • The author didn't have commit access

@DavidSpickett
Copy link
Collaborator Author

I am not sure if it is possible to make a bot that suggests this when

I'm going to find out! - https://discourse.llvm.org/t/rfc-encouraging-reviewers-to-merge-on-behalf-of-pr-authors-without-commit-access/84180/14?u=davidspickett

@DavidSpickett DavidSpickett merged commit 4b6fc49 into llvm:main Jan 24, 2025
6 of 8 checks passed
@DavidSpickett DavidSpickett deleted the llvm-prs branch January 24, 2025 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants