Page MenuHomePhabricator

add a custom field "commit in gerrit" in bugzilla ui.
Closed, DeclinedPublic

Description

please add this field for easy tracking of bugs that has patches in gerrit, and might even be used one day as a field id bug#17322 is fixed.

This is done by going to: administration --> custom fields-->Add a new custom field.

Thanks.


Version: unspecified
Severity: enhancement

Details

Reference
bz39399

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 12:52 AM
bzimport set Reference to bz39399.
bzimport added a subscriber: Unknown Object (MLST).

I don't have an experience of administration of Bugzilla but what should it be? A comma-separated list of integer with automatic URL display?

no, just a free text box. so it can display a hash or a url.

(In reply to comment #2)

no, just a free text box. so it can display a hash or a url.

So it's unclickable and I have to copy-paste it manually or find a link in comments?

it is clickable exactly like the field see also.

I'm not sure what the point of this is..

Suggesting wontfix.

We have too many input fields already. Just put it in a comment. That makes it readable, clickable and it is easy to see when it was added by whom, what the context is (message). and old links are still discoverable in the main thread as well (instead of back in the "Modify history").

Putting it in a comment also allows usage of git hashes or gerrit change ids, which auto-link.

I'm slightly in favor of this. Currently it's hard for me to see for which tickets there is a patch available, as patches are handled in Gerrit and not in Bugzilla (and as the four "patch" keywords are not very useful and errorprone).
A specific field would let me see immediately if a related code change for the reported problem is available, plus it would be queryable via Bugzilla's custom search ("Gerrit ID | does not match regular expression | ^$").
I agree that it should behave like the "See Also" field though (clickable link) which likely requires some code changes. :/

As mentioned before, storing it would be a degration in interface flexibility for no apparent purpose.

We already have keywords to indicate there being a proposed patch:

  • patch (patch attached to bugzilla)
  • patch-in-gerrit (patch submitted for review on gerrit)

I consider these keywords rather useless and unhandy. Reopening.

(In reply to comment #11)

I consider these keywords rather useless and unhandy. Reopening.

Please be more elaborate, this comment is not helping. keywords can be indexed, added with auto-completion, searched, tracked. What else?

The four keywords very likely predate Gerrit.
"patch-in-gerrit" - without reading the description you don't know that you should remove this once the patch is *reviewed* in gerrit. It contains no information whether the review was positive or negative.
"patch-need-review": Does not provide any info where the review is expected to take place. Bugzilla, Gerrit?
"patch-reviewed": What does it mean at all and why is that interesting in Bugzilla in case the review took place in Gerrit? In case it's kind of relevant in Bugzilla, it does not tell you if the patch was accepted or not accepted.

I've seen removal of the "patch" keyword together with removal of "patch-in-gerrit" and replacement by "patch-reviewed". What is the "patch" keyword there for at all if basically means the same as "patch-need-review" initially?

General problem: Keywords are per bug report. A bug can have several patches attached and it's unclear what the keywords refer to. Last patch? All patches?

Sounds rather like a usecase for a flag. Use the "patch" checkbox for patches (no need to duplicate info by a keyword), set review? flag to request review, get review+ or review- as feedback. All of course only if Bugzilla was used instead of Gerrit.

Afaik the current workflow is as follows (regardless of likely-outdated documentation):

  • Keyword usage on open bugs:

"patch" means there is a proposed patch that (may) fix this bug. Depending on review status, it may or may not be reviewed yet, but presence of this patch should indicate that there is no need for a new patch by a new participant (the existing participants will proceed review/improvement/submit/merge process, and if it leads to a dead-end the patch is marked obsolete and the keywords removed).

For patches in bugzilla "patch" is used for indicating presence and "patch-reviewed" as way to review it. For patches in gerrit "patch-in-gerrit" is used for indicating presence of the patch and review status is handled on Gerrit instead.

As far as I'm concerned we could merge "patch" and "patch-in-gerrit" as they indicate the same thing for as far as we care for queryability.

The "patch" checkbox in the attachment handling is interesting, if that can be queried then that is probably superior than using a keyword, however then one would have to OR search between "attachments not marked obsolete with 'patch'-flag" and "keyword patch-in-gerrit".

"patch-need-review" is obsolete imho with patch && !patch-reviewed.

"patch-need-review" and "patch-reviewed" are only used for bugzilla patches.

  • Keyword usage on resolved bugs is not consistent. Some remove then, some leave them. Either way, in queries we always usually exclude resolved bugs anyway.

The original request for a custom field is WONTFIX, the plan is to go with http://lists.wikimedia.org/pipermail/wikitech-l/2013-June/069805.html instead.