-
-
Notifications
You must be signed in to change notification settings - Fork 60
Add the link to b.p.o as a comment on GitHub #3
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
Comments
Or, instead of a comment, maybe just update the PR description with a link to b.p.o. |
On Thu, 13 Apr 2017 03:40:48 -0700, Mariatta ***@***.***> wrote:
Or, instead of a comment, maybe just update the PR description with a link to b.p.o.
I like this, since it would put the link at the top of the page in a
predictable place.
|
agreed. a comment containing the full URL is much more obvious to humans
than knowing we should look for a "Details" link from something named
bedevere. :)
On Thu, Apr 13, 2017 at 6:23 AM R. David Murray <[email protected]>
wrote:
… On Thu, 13 Apr 2017 03:40:48 -0700, Mariatta ***@***.***>
wrote:
> Or, instead of a comment, maybe just update the PR description with a
link to b.p.o.
I like this, since it would put the link at the top of the page in a
predictable place.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAELi7xRqczdOQaZ4S5CpL5YwmHFNSsVks5rviHkgaJpZM4M8iGo>
.
|
So I take it people want both the check so people know they should add the issue number and a comment that links back to the issue? Or at people arguing for dropping the status check entirely in favour of just a comment? One trick with adding the comment is the bot will need to check if a comment was already left before leaving a new one. There's also editing in-place the first comment from the bot with the link in case the issue number is changed. IOW it's a bit more complicated than the current solution, hence why I did what I did. 😄 |
I think the status check is still useful, and it's already implemented, why not keep it. Instead of creating a new comment, is it possible to update the PR description? If the issue number is changed, simply replace it. |
If you're talking about after a PR is closed then that's fine, although at that point what's the benefit if you're still going to have to copy and paste the link compared to the issue number from the title? The "bpo-" prefix namespaces the number so I'm not seeing the benefit of specifying the full URL for record-keeping. Is this purely for people who have no idea what bpo numbers represent? Otherwise you're basically after the comment for the convenience of the link to click on (that's what @gpshead seems to be advocating for). The status check exists because people seemed to want that in order to get new contributors to make sure they didn't leave it off. But if people want a comment instead that's fine, it just needs to be agreed upon as I don't want to write this code a third time 😉 . Otherwise I'm hearing from some people they want a status check to make sure the issue number is there while others are simply asking for a link out of convenience. And we can have both, but since both approaches have ramifications (i.e. notification of a comment versus a failing PR check) I want to make sure people are upfront about what exactly they are after. |
To help get a clearer and wider amount of feedback to settle this conclusively I have emailed core-workflow on this topic. |
I would like both a status check and a persistent link. The status check should be for either bpo-nnnn in the title or a Trivial tag. Should only committers be able to tag? I would like 'bpo-nnnnn' in the title to be the link, but I am guessing this is not possible as we do not control everything on the PR. |
I want both the status check and the convenience link. If I had to
choose only one I'd pick the convenience link, but not by a wide margin.
|
@terryjreedy I believe only people with write access to the repo can set labels, so you already have the control level you want. |
Just an FYI, my current thinking is we will append the link to the PR message's body along with some text reminding folks to keep comments on GitHub to only code reviewing, while all other comments should go to bugs.python.org. |
That sounds good. |
/cc |
This is next in the work queue for me unless someone else wants to tackle it, in which case I will move on to the next work item. |
@brettcannon I will work on this tomorrow after I wake up (unless you are already working on it). |
Nope, go for it! |
I should mention an idea I had was to use HTML comments to surround the inserted text to make it easy to discover if the link had already been inserted or something:
|
Currently, the link back to b.p.o is only shown as a status check when PR is not yet merged.
Often I still re-read a merged PR, for example while backporting changes.
After a cherry-pick PR has been merged, we're supposed to go back to the bug tracker and close the issue.
However, since the PR has been merged, the status check is no longer visible.
So I think it will be nice if the link to the bug tracker can be added as a GitHub comment that is always going to be available.
What do you think about this?
The text was updated successfully, but these errors were encountered: