Page MenuHomePhabricator
Create Task
Maniphest T76291

gerritbot should report both author and committer
Closed, ResolvedPublic
Actions

Description

E.g.

Change 166989 had a related patch set uploaded (by Merlijn van Deen):
UnicodeDecodeError in pwb.py creating logfile

https://gerrit.wikimedia.org/r/166989

suggests I'm the one who wrote the patch, even though that's not the case. Maybe we can even just add the full commit message, e.g.

Change 166989 had a related patch set uploaded:

Author: John Vandenberg
Committer: Merlijn van Deen

UnicodeDecodeError in pwb.py creating logfile

Use the decoded copy of the command line arguments.

Bug: T74129
Change-Id: I8b6d22ca1c1db323763efb2ce2d8825158bc8b7e

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Gerrit: Fix bot so that it checks against *-name and *-usernameoperations/puppetproduction+3 -3
Gerrit: only show author and uploader if they are different peopleoperations/puppetproduction+5 -0
Gerrit: Make GerritBot report author of patch and the uploader of patchoperations/puppetproduction+1 -1
Customize query in gerrit

Related Objects

Event Timeline

valhallasw created this task.Nov 30 2014, 2:53 PM
valhallasw raised the priority of this task from to Needs Triage.
valhallasw updated the task description. (Show Details)
valhallasw added projects: Gerrit, Phabricator.
valhallasw changed Security from none to None.
valhallasw subscribed.
XZise subscribed.Nov 30 2014, 4:09 PM

I don't think it's good to add the complete commit messages, because some are very long. Although on second thought that message would only appear once.

With T873 implemented the link wouldn't be necessary by the way.

Qgil triaged this task as Low priority.Dec 2 2014, 8:06 AM
Qgil subscribed.
Ricordisamoa subscribed.Dec 13 2014, 1:12 PM
Qgil lowered the priority of this task from Low to Lowest.Dec 22 2014, 9:37 AM
Qgil added a project: Gerrit-Migration.

This has been the status quo for a long time. It doesn't really matter who uploaded/created the patch, the call for action is that there is a new patch for review.

I'd rather focus our attention in migrating #Code-Review, and keep the work on the current Gerrit-Phabricator setup to a minimum. Proposing to decline this task.

Qgil moved this task from To Triage to Need discussion on the Phabricator board.Dec 22 2014, 9:38 AM
Krenair subscribed.Feb 21 2015, 4:01 AM
jayvdb subscribed.Mar 20 2015, 6:58 AM
Tgr subscribed.Jun 18 2015, 5:11 PM

This has been the status quo for a long time.

It has been annoying/confusing for a long time :-)

How about

Change 166989 had a related patch set uploaded (by Merlijn van Deen; onwer: John Vandenberg):

?

(Note that the commit author and the gerrit owner are not necessarily the same, although in practice they almost always are.)

Restricted Application added a subscriber: scfc. · View Herald TranscriptJun 18 2015, 5:11 PM
jayvdb awarded a token.Jun 19 2015, 3:17 AM
greg removed a project: Gerrit-Migration.Sep 9 2015, 5:37 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 9 2015, 5:37 PM
Danny_B edited projects, added GerritBot; removed Gerrit.May 19 2016, 3:00 PM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMay 19 2016, 3:00 PM
Aklapper moved this task from Need discussion to Misc on the Phabricator board.Oct 7 2016, 4:44 PM
Aklapper removed a project: Phabricator.Oct 7 2016, 4:49 PM
Paladox subscribed.Feb 26 2017, 9:20 PM
gerritbot subscribed.Feb 26 2017, 9:26 PM

Change 339980 had a related patch set uploaded (by Paladox):
Gerrit: Make GerritBot report author of patch and the uploader of patch

https://gerrit.wikimedia.org/r/339980

gerritbot added a project: Patch-For-Review.Feb 26 2017, 9:26 PM
Paladox added a comment.Feb 26 2017, 11:59 PM

I've tested ^^ and it works here https://phab-01.wmflabs.org/T20#2762 :)

Dzahn raised the priority of this task from Lowest to Medium.Feb 27 2017, 5:45 PM
gerritbot added a comment.Feb 27 2017, 5:49 PM

Change 339980 merged by Dzahn:
Gerrit: Make GerritBot report author of patch and the uploader of patch

https://gerrit.wikimedia.org/r/339980

Dzahn subscribed.Feb 27 2017, 5:53 PM
In T76291#939374, @Qgil wrote:

keep the work on the current Gerrit-Phabricator setup to a minimum. Proposing to decline this task.

This has been said in 2014. Proposing to keep improving Gerrit setup and resolving instead of declining.

Dzahn added a comment.Feb 27 2017, 5:55 PM

Tested on T137928#3058693. Works! "Change 339763 had a related patch set uploaded (by Dzahn; owner: Paladox):"

Thanks for the fix, @Paladox

Dzahn closed this task as Resolved.Feb 27 2017, 5:56 PM
Dzahn assigned this task to Paladox.
Paladox removed a project: Patch-For-Review.Feb 27 2017, 5:58 PM
Tgr added a comment.Feb 28 2017, 2:02 AM

Filed T159202: gerritbot should report which repository a patch is for which is pretty similar.

Ricordisamoa added a comment.Feb 28 2017, 3:20 AM

Please don't report it twice if the owner and uploader are the same :)

Paladox added a comment.Feb 28 2017, 8:28 AM
In T76291#3060167, @Ricordisamoa wrote:

Please don't report it twice if the owner and uploader are the same :)

I'm not sure if that's possible.

Paladox added a comment.Feb 28 2017, 4:03 PM
In T76291#3060167, @Ricordisamoa wrote:

Please don't report it twice if the owner and uploader are the same :)

Actually it's possible, i have done it here https://gerrit.wikimedia.org/r/#/c/340326/ :), also tested and works.

gerritbot added a comment.Feb 28 2017, 4:03 PM

Change 340326 had a related patch set uploaded (by Paladox; owner: Paladox):
Gerrit: Fix it so that if you are the author and uploader it will only show By:

https://gerrit.wikimedia.org/r/340326

gerritbot added a project: Patch-For-Review.Feb 28 2017, 4:03 PM
Paladox added a comment.Feb 28 2017, 4:04 PM
In T76291#3058695, @Dzahn wrote:

Tested on T137928#3058693. Works! "Change 339763 had a related patch set uploaded (by Dzahn; owner: Paladox):"

Thanks for the fix, @Paladox

Your welcome :)

gerritbot added a comment.Feb 28 2017, 7:18 PM

Change 340326 merged by Dzahn:
Gerrit: only show author and uploader if they are different people

https://gerrit.wikimedia.org/r/340326

Dzahn added a comment.Feb 28 2017, 7:38 PM
In T76291#3060167, @Ricordisamoa wrote:

Please don't report it twice if the owner and uploader are the same :)

This should be fixed now :)

Ricordisamoa added a comment.Mar 2 2017, 6:44 PM
In T76291#3062120, @Dzahn wrote:
In T76291#3060167, @Ricordisamoa wrote:

Please don't report it twice if the owner and uploader are the same :)

This should be fixed now :)

It doesn't work when Git and Gerrit usernames differ (despite email addresses being the same) as in T159075#3065963

Paladox added a comment.Mar 2 2017, 7:10 PM

Oh i see thats because theres two types of configs in its-base (more the ones i will be showing in this example)

there's

author-name
author-username
uploader-name
uploader-username

gerritbot added a comment.Mar 3 2017, 4:04 AM

Change 340801 had a related patch set uploaded (by Dzahn; owner: Paladox):
[operations/puppet] Gerrit: Fix bot so that it checks against *-name and *-username

https://gerrit.wikimedia.org/r/340801

gerritbot added a comment.Mar 3 2017, 4:06 AM

Change 340801 merged by Dzahn:
[operations/puppet] Gerrit: Fix bot so that it checks against *-name and *-username

https://gerrit.wikimedia.org/r/340801

Dzahn mentioned this in T159441: gerritbot reports changesets as "merged by" change owner, even though they are merged by other people.Mar 3 2017, 4:13 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits