[GitHub] Allow shortcut for "introductory issue" and request linking to issue in PR #84635
Conversation
|
Personally, I prefer the way of having a clear assignee to "introductory issue" tickets. |
|
This weekend we hit a problem on this very topic - a good-first-issue ticket had been claimed and assigned to somebody, who had been working on a patch but hadn't shown anything after a few days. Another contributor took the 'first to PR' approach and the PR was reviewed and had begun to be accepted before we realised that it wasn't from the original assignee. The conclusion was that the original assignee was given the chance to create their own PR for review, which they took and the earlier PR was closed, unfinished and uncommitted. Such wasted work, and bad feeling that was created, is something we need to avoid - we need issue assignments to be clear and respected. If an assigned issue looks to be too quiet, I recommended we keep to the 'ping after 7 days' guide we've used in the past for reviews instead of making wasteful assumptions. I've been creating a number of good-first-issue tickets recently; they tend to be smaller/simpler in scope so more seasoned committers are likely to be able to put them together very quickly, which can be unfortunate if we're trying to encourage newcomers to take them on instead, as they will probably take a little longer to create a working patch and then upload the PR. They need the assignment to make it clear they are working on this. |
|
That seems to be handled differently in different parts of the project. In the clang-tidy area, for instance, I often see the "just create a PR" comments to avoid bothering someone to assign people. Especially for tickets lying around untouched for a while, this is a typical advice. Two people working on the same topic is unlikely then. That's what brought me to this update. I get your points, but also wonder if not both approaches are valid. The issues we are talking about are already tagged as "good first issue", so more experienced contributors would rather leave them for newcomers anyway. Now, asking for assignment is of course valid. Then the matter is clear. But if nobody got assigned and someone prepared a PR already, why not accept it as well? By referencing the issue in the PR, the PR appears in the issue and that would serve as an assignment. Therefore, I added this "mention the issue in the PR description" part. |
SimplyDanny
force-pushed
the
update-introductory-issue-text
branch
from
277994f
to
60249b9
Compare
| 5. Run [`git clang-format HEAD~1`](https://clang.llvm.org/docs/ClangFormat.html#git-integration) to format your changes. | ||
| 6. Open a [pull request](https://github.com/llvm/llvm-project/pulls) to the [upstream repository](https://github.com/llvm/llvm-project) on GitHub. Detailed instructions can be found [in GitHub's documentation](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request). | ||
| 1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed. | ||
| 1. In the comments of this issue, request for it to be assigned to you, or just create a [pull request](https://github.com/llvm/llvm-project/pulls) after following the steps below. [Mention](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this issue in the description of the pull request. |
There was a problem hiding this comment.
Should we explicitly say that raising a PR for an issue assigned to somebody else might lead to your PR being rejected? We should perhaps get some advice from other people on this as we're creating policy here.
There was a problem hiding this comment.
The updated version doesn't sound too harsh. It's just friendly advice, no real policy so far. In the (expectedly rare) situation of a PR being created even though another person is assigned, some balancing would be required. Given the initial kind advice, it's clear that violating it is not something that will be accepted silently without explicitly writing it down.
In case you still think, this requires other opinions, who would these people be?
There was a problem hiding this comment.
@pogo59 Do you have any suggestions on this please?
There was a problem hiding this comment.
Given that the suggestion is to ping and wait a week if the issue is assigned to someone else, the risk of competing PRs is really low. And even it happens, the general "be nice" directive from the Code of Conduct would apply. I think it's fine as is.
…to issue in PR (llvm#84635) The answer to many requests in issues to be assigned to users is often "just create a pull request". That's in contradiction to the "introductory issue" instructions posted by the GitHub bot. This change updates the instructions, mentioning the shortcut of "just creating a PR". Moreover, it now explains linking PRs to issues in order to close them automatically upon merge.
The answer to many requests in issues to be assigned to users is often "just create a pull request". That's in contradiction to the "introductory issue" instructions posted by the GitHub bot.
This PR updates the instructions, mentioning the shortcut of "just creating a PR". Moreover, it now explains linking PRs to issues in order to close them automatically upon merge.