Page MenuHomePhabricator

Warn that patches should go to Gerrit instead of Bugzilla
Closed, ResolvedPublic

Description

Show warning in "Add an attachment" that patches should go to Gerrit instead.


Version: wmf-deployment
Severity: enhancement
Whiteboard: gci2013 https://www.mediawiki.org/wiki/Google_Code-In#Candidate_tasks
URL: https://google-melange.appspot.com/gci/task/view/google/gci2013/6471850532012032

Details

Reference
bz42606

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:10 AM
bzimport set Reference to bz42606.
bzimport added a subscriber: Unknown Object (MLST).

Most patches should go in Bugzilla (screenshots, pastebins, etc.).

Patches to code that's hosted in Git should go in Gerrit, in theory. If the user is willing/able to make a Gerrit account and deal with Gerrit.

(In reply to comment #1)

Most patches should go in Bugzilla (screenshots, pastebins, etc.).

Screenshots are not patches.

(In reply to comment #2)

(In reply to comment #1)

Most patches should go in Bugzilla (screenshots, pastebins, etc.).

Screenshots are not patches.

Sorry, you're right. I was reading "attachment" and "patch" as synonymous. It's too early and I'm not yet awake.

I'd consider the lack of self-registration a blocker to resolving this bug. That is, a user can't just make a Gerrit account and submit a changeset (currently) in the same way that he or she can create a Bugzilla account and submit a patch.

And if a warning is implemented when submitting patches to Bugzilla encouraging the use of Git/Gerrit, I think an even larger notice will be need to be implemented somewhere warning of the awfulness of Gerrit. ;-)

(In reply to comment #4)

I'd consider the lack of self-registration a blocker to resolving this bug.

There's no sense in having patches in different places like Bugzilla or mailing lists when they are expected in Gerrit. That's why we should tell people to go to Gerrit (and get Developer Access). I don't plan to shut down attaching attachments in Bugzilla. I just want to clarify it's the wrong place for patches. Once a hint is in place this ticket is resolved - unrelated to Gerrit's account process and its flaws.

Given the hurdles required to get into, and then get code out of, and then get patches into gerrit (and that's assuming the framework doesn't crap out and completely fail on you like it just did for me for what should have been an incredibly simple change), such a warning seems like it would be more likely to put people off bothering to write patches at all than to try to put them in a more centralised location.

Seriously, considering just how completely terrible gerrit is, anything more than a notice that it's something to consider as a more direct alternative (and thus more likely to get merged) seems most unwise.

The way out is to fix Gerrit, not to abuse another place (Bugzilla) to drop stuff there that should go to the correct place (Gerrit).
Plus it's no argument to not tell people to go to Gerrit because that's the place where patches should go. (I didn't write "must go".) Full stop.

I second the Andre Klapper comment.

Phabricator for example offers you two way to push your code in the review system:

  • arc diff (from a user perspective, that works like our git review, ie that will send the proposed change to the review queue)
  • a plain textarea to copy/paste the patch inside

In Gerrit world, this means we would have a plugin to process a form and then:

  • create a new branch in the right repository
  • apply the patch
  • commit the change

The main issue is this would still require a developer access.


Here a story from a one-time contributor who wanted to submit a patch the proper way:
http://blogs.gnome.org/bolsh/2012/09/14/attention-speed-bump-ahead/

[ Taking this bug by the way ]

(In reply to comment #8)

[ Taking this bug by the way ]

Dereckson: Do you still plan to work on this?
Any pointers or hepl that I could offer?

(In reply to comment #8)

[ Taking this bug by the way ]

Dereckson: Do you still plan to work on this?
Any pointers or help that I could offer?

No feedback => I am unassigning this report and I've turned this into a Google Code-In task.

t.lam wrote:

Would adding this "Warning: Patches should go on Gerrit if possible" above the patch checkbox in the Content Type field be acceptable?

Change 96684 had a related patch set uploaded by Tholam:
Add warning that patches should go to Gerrit instead of Bugzilla

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

Now that we have https://tools.wmflabs.org/gerrit-patch-uploader/ which allows anyone to easily upload a patch from bugzilla to gerrit, is this still needed?

At least for Pywikibot, some people have issues with gerrit, so we'll take patches anywhere.

legoktm: Hmmm, good point (as usual).
Maybe linking to https://www.mediawiki.org/wiki/Developer_access instead of gerrit.wikimedia.org, and making sure that the Developer_access documentation also mentions the gerrit-patch-uploader as an alternative?

t.lam wrote:

Changed tab to spaces but git doesn't think i've made any changes so can't upload to gerrit

Attached:

What procedure did you followed to update the change?

You can for example do the following :

1 - go to the git branch where the code is

git checkout bug/42606

2 - edit the file

3 - Amend the previous commit and send modified version to Gerrit

git add createformcontents.html.tmpl
git commit --amend
git review -R

Change 96684 merged by Dzahn:
Add warning that patches should go to Gerrit instead of Bugzilla

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

i see the warning now when adding attachment on this bug: Note: Patches should go into Gerrit if possible.

lgtm