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.

Sign up for GitHub

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

Jump to bottom

x/build/cmd/gerritbot: leave a CL comment on a freshly imported PR to help confirm the author has a Gerrit account #61316

Open
thepudds opened this issue Jul 12, 2023 · 3 comments · May be fixed by golang/build#69
Open

x/build/cmd/gerritbot: leave a CL comment on a freshly imported PR to help confirm the author has a Gerrit account #61316

thepudds opened this issue Jul 12, 2023 · 3 comments · May be fixed by golang/build#69
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@thepudds
Copy link
Contributor

thepudds commented Jul 12, 2023 •
edited
Loading

In #61182, Bryan has an excellent list of friction around the GitHub PR workflow, including:

A significant fraction of GitHub PRs are abandoned when the author does not respond to Gerrit comments, but the existence of an open PR discourages other contributors from working on the corresponding issue.

One minor thing we could do to shave off some friction is leave a welcome comment in Gerrit after a PR is imported, wherein we:

  • ask the PR author to respond to the comment in Gerrit confirming the CL is ready for review, and
  • provide some context to the possibly new contributor, including a link to the Review section of the contributing guide

If they respond to the comment, a potential reviewer then knows that the author:

  • has a Gerrit account, and
  • knows how to reply in Gerrit

To make this suggestion more concrete, I will send a draft CL shortly.

Possible extensions

A modest extension would be to have a reminder comment after a week or so if the author does not reply. This could include some hints around the reply mechanism, including to help with the problem of unpublished Gerrit draft responses.

Another extension would be to automatically abandon the CL and close the PR with an explanatory message if there is no reply after a month or so. This would help with the problem of an open PR discouraging other contributors.

Both of these possible extensions are left as TODOs in the code and possible future issues.
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Jul 12, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 12, 2023
This was referenced Jul 12, 2023
Closed
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/509135 mentions this issue: cmd/gerritbot: leave a CL comment on a freshly imported PR to help confirm the author has a Gerrit account

@thepudds thepudds changed the title x/build/cmd/gerritbot: leave a CL comment for new contributors to help confirm they have a Gerrit account x/build/cmd/gerritbot: leave a CL comment on a freshly imported PR to help confirm the author has a Gerrit account Jul 12, 2023
@thepudds
Copy link
Contributor Author

Sent a rough CL 509135 to help discussion.

CC @heschi, @cagedmantis

@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 13, 2023
@thepudds thepudds mentioned this issue Jul 25, 2023
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/513397 mentions this issue: cmd/gerritbot: automatically flag common issues in PRs

gopherbot pushed a commit to golang/build that referenced this issue Aug 15, 2023
@thepudds @gopherbot
cmd/gerritbot: automatically flag common issues in PRs
fdf5df9 
In this CL, we add a set of checks that automatically run
against a new GitHub PR after it is imported into Gerrit
in order to flag common issues, such as missing a bug
reference or ending the first line of the commit with
a period. See below for the current set of defined rules.

The intent is to save maintainer time, as well as
perhaps make a better initial experience for a
new contributor via faster feedback with potentially
helpful linked resources.

By attempting to engage a new contributor as soon as possible
along with some pointers, we also hope to get them
to reply (successfully) in Gerrit, which in turn helps
a potential reviewer see that the contributor knows
how to use Gerrit.

The package comment in rules.go provides an overview
of the implementation approach.

The rules themselves are defined as short functions
in rules.go.

We currently have 100% test coverage within the rules package.

Sample results from running on a corpus of 1000 GitHub PRs
that were imported into Gerrit:

  Avg. findings per CL: 1.42
  CLs with findings:    74.5%

  CL hit %   Finding
  --------   ------------------------------------------------
      9.9%   title: no package found
      0.1%   title: no colon then single space after package
     13.0%   title: no lowercase word after a first colon
      2.0%   title: ends with period
     20.8%   body: short
      6.1%   body: no sentence candidates found
     21.7%   body: long lines
     10.5%   body: might use markdown
      2.2%   body: still contains PR instructions
      0.9%   body: contains Signed-off-by
     43.9%   body: no bug reference candidate found
      9.3%   body: bug format looks incorrect
      1.5%   body: no bug reference candidate at end

See golang/go#61573 for additional background.

Updates golang/go#61316
Fixes golang/go#61573

Change-Id: I866cf650608d6ce9c6783aabc17a219f1815908c
Reviewed-on: https://go-review.googlesource.com/c/build/+/513397
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: t hepudds <thepudds1460@gmail.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Release
Status: Planned
Milestone
Unreleased
3 participants
@gopherbot @cherrymui @thepudds