Hello,
Recently I noticed that keywords in bugzilla get updated more and more often, mostly with keywords like "patch", "patch-need-review", etc.
I am wondering what to do in the following situations (like https://bugzilla.wikimedia.org/show_bug.cgi?id=39635 for example):
- user A posts a patch - the bug gets "patch", "patch-need-review" - user B posts a patch that is different and says he does not like patch of A - user B submits change to gerrit
When "need-review" should be removed? What are replacements if any? What if I believe that core ideas behind the patch are wrong? What if I just think the implementation should be improved? What it it's more or less okay? I see only "patch-reviewed" in the keywords - which can be both negative and positive.
Before I open a whole can of worms by asking a question how do I relate those keywords to the Gerrit workflow we have, maybe the current bugmeisters could explain how they use those keywords and how we can help?
//Saper
*
On 08/27/2012 01:59 PM, Marcin Cieslak wrote:
- user A posts a patch
- the bug gets "patch", "patch-need-review"
- user B posts a patch that is different and says he does not like patch of A
- user B submits change to gerrit
When "need-review" should be removed?
User B should remove "need-review". Code in gerrit has its own review process and it shouldn't be necessary to keep the keywords in Bugzilla up to date. User B should make sure that the bug has a comment referring to his gerrit submission.
What if I believe that core ideas behind the patch are wrong?
Leave a comment with your thoughts. You could also remove the "need-review" keyword and, optionally, mark the patch as obsolete. Marking it as obsolete is a pretty strong statement, though.
What if I just think the implementation should be improved?
Remove the "need-review" and leave a comment with your suggestions. Directing the submitter to gerrit for future submissions is a good idea, too, but that might add another step that that makes future submissions improbable.
What it it's more or less okay?
Remove the "need-review" keyword and direct the submitter to gerrit or submit it to gerrit on their behalf. Make sure you refer to the bug number in first line of the commit message (see https://gerrit.wikimedia.org/r/#/c/13855/ for an example). Other people will then have a chance to review it and it may be merged.
If you don't have a gerrit account, apply for one or find someone who does to apply the patch.
Before I open a whole can of worms by asking a question how do I relate those keywords to the Gerrit workflow we have
Oops! I guess I opened the can and not you!
Mark.
See also https://www.mediawiki.org/wiki/Thread:Talk:Git/Workflow/Bugzilla
Best regards, Helder
On Mon, 2012-08-27 at 17:59 +0000, Marcin Cieslak wrote:
When "need-review" should be removed?
If I understand it correctly: If the review takes place in Gerrit anyway, then I don't see much sense in having this keyword in Bugzilla. It should rather indicate something like "The attached patch needs be moved to Gerrit so it can get a review in Gerrit".
I see only "patch-reviewed" in the keywords - which can be both negative and positive.
Apart from whether it makes sense to have this keyword if patch review takes place in Gerrit anyway:
"patch-reviewed" obviously refers to a patch, while keywords refer to a report. Hence using a keyword does not work well if a report includes more than one patch. This sounds like a classic usecase for a Bugzilla flag: Set "review?" for requesting a review for a patch, get "review+" for acceptance or "review-" for refusal.
andre
Personally I don't like these keywords in bugzilla __at all__ and don't use them. This is mostly because I think they are extremely hidden in the interface.
From my point of view bugzilla should provide a status: 'in review'
alongside 'new', 'unconfirmed', 'resolved' and 'assigned'
This would be useful for cases where a pull request has been sent or a patch attached - so effectively the ticket has almost been resolved pending this last review step.
I find the status filter much more useful and would allow me to easily see what bugs needs review. It also helps me filter the bug list to attempt to solve things that haven't been tackled.
If I review a patch or a pull request and don't think it's suitable then I would propose that we mark it as REOPENED. This at least signals to the provider of the patch that more work needs to be done.
Thoughts?
On Mon, Aug 27, 2012 at 10:59 AM, Marcin Cieslak saper@saper.info wrote:
Hello,
Recently I noticed that keywords in bugzilla get updated more and more often, mostly with keywords like "patch", "patch-need-review", etc.
I am wondering what to do in the following situations (like https://bugzilla.wikimedia.org/show_bug.cgi?id=39635 for example):
- user A posts a patch
- the bug gets "patch", "patch-need-review"
- user B posts a patch that is different and says he does not like patch of A
- user B submits change to gerrit
When "need-review" should be removed? What are replacements if any? What if I believe that core ideas behind the patch are wrong? What if I just think the implementation should be improved? What it it's more or less okay? I see only "patch-reviewed" in the keywords - which can be both negative and positive.
Before I open a whole can of worms by asking a question how do I relate those keywords to the Gerrit workflow we have, maybe the current bugmeisters could explain how they use those keywords and how we can help?
//Saper
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
+1.
-Chad On Aug 28, 2012 6:24 PM, "Jon Robson" jdlrobson@gmail.com wrote:
Personally I don't like these keywords in bugzilla __at all__ and don't use them. This is mostly because I think they are extremely hidden in the interface.
From my point of view bugzilla should provide a status: 'in review' alongside 'new', 'unconfirmed', 'resolved' and 'assigned'
This would be useful for cases where a pull request has been sent or a patch attached - so effectively the ticket has almost been resolved pending this last review step.
I find the status filter much more useful and would allow me to easily see what bugs needs review. It also helps me filter the bug list to attempt to solve things that haven't been tackled.
If I review a patch or a pull request and don't think it's suitable then I would propose that we mark it as REOPENED. This at least signals to the provider of the patch that more work needs to be done.
Thoughts?
On Mon, Aug 27, 2012 at 10:59 AM, Marcin Cieslak saper@saper.info wrote:
Hello,
Recently I noticed that keywords in bugzilla get updated more and more often, mostly with keywords like "patch", "patch-need-review", etc.
I am wondering what to do in the following situations (like https://bugzilla.wikimedia.org/show_bug.cgi?id=39635 for example):
- user A posts a patch
- the bug gets "patch", "patch-need-review"
- user B posts a patch that is different and says he does not like patch of A
- user B submits change to gerrit
When "need-review" should be removed? What are replacements if any? What if I believe that core ideas behind the patch are wrong? What if I just think the implementation should be improved? What it it's more or less okay? I see only "patch-reviewed" in the keywords - which can be both negative and positive.
Before I open a whole can of worms by asking a question how do I relate those keywords to the Gerrit workflow we have, maybe the current bugmeisters could explain how they use those keywords and how we can help?
//Saper
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
-- Jon Robson http://jonrobson.me.uk @rakugojon _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Jon Robson wrote:
Personally I don't like these keywords in bugzilla __at all__ and don't use them. This is mostly because I think they are extremely hidden in the interface.
From my point of view bugzilla should provide a status: 'in review' alongside 'new', 'unconfirmed', 'resolved' and 'assigned'
It'd be nice if the statuses weren't all shouted and single words. INREVIEW is pretty unpleasant.
This would be useful for cases where a pull request has been sent or a patch attached - so effectively the ticket has almost been resolved pending this last review step.
Right. We've been hitting this more and more lately. It used to be that you'd commit to SVN, mark the bug as fixed, and then if your revision was reverted, the bug would be reopened by the person doing the reverting. With Git, the development workflow has changed, so it makes sense to adjust Bugzilla's workflow as necessary.
I find the status filter much more useful and would allow me to easily see what bugs needs review. It also helps me filter the bug list to attempt to solve things that haven't been tackled.
You should be able to just as easily filter by keyword, but this isn't to suggest that I disagree with your broader points (more below).
If I review a patch or a pull request and don't think it's suitable then I would propose that we mark it as REOPENED. This at least signals to the provider of the patch that more work needs to be done.
Thoughts?
I think we should take a holistic approach to the Bugzilla workflow. I was hoping the incoming Wikimedia Foundation entomologist would work on this. It'd be great to fix one aspect of bug filing (such as the use of keywords), but it'd be even better to take a look at the entire workflow and figure out ways to make it suck less.
For example, the high-level categorizations are pretty awful currently. What is and is not a product is inconsistent and confusing. I started some notes about this at https://www.mediawiki.org/wiki/Requests_for_comment/Bugzilla_taxonomy.
Perhaps that page could/should be re-titled to "Bugzilla workflow" and we could address a few outstanding Bugzilla workflow problems at once?
MZMcBride
From my point of view bugzilla should provide a status: 'in review' alongside 'new', 'unconfirmed', 'resolved' and 'assigned'
It'd be nice if the statuses weren't all shouted and single words. INREVIEW is pretty unpleasant.
How about ATTEMPTED, PATCHED, DEVELOPED, PROPOSED or even FIXED ? The other way to look at it is to introduce "RESOLVED / VERIFIED" which would allow although I'd prefer a separate state for backwards compatibility. That way RESOLVED FIXED means - I fixed this but it's not the end of the story yet.. FIXED might cause confusion because of it's use in resolved fixed and seems a bit presumptuous.
This would be useful for cases where a pull request has been sent or a patch attached - so effectively the ticket has almost been resolved
pending
this last review step.
Right. We've been hitting this more and more lately. It used to be that you'd commit to SVN, mark the bug as fixed, and then if your revision was reverted, the bug would be reopened by the person doing the reverting. With Git, the development workflow has changed, so it makes sense to adjust Bugzilla's workflow as necessary.
+1 I am a big offender of this - and the reason I do it is I want to
filter it from my list of open bugs and there is no in between step.
I would like to stress the more natural way to review patches in Gerrit seems to be directly from the Gerrit site, making less useful the possibility to easily search on Bugzilla patch with code in Gerrit.
Is one of you has a real use case with Bugzilla as gerrit patch finding tool?
-- Dereckson
On Tue, Aug 28, 2012 at 3:48 PM, MZMcBride z@mzmcbride.com wrote:
I think we should take a holistic approach to the Bugzilla workflow. I was hoping the incoming Wikimedia Foundation entomologist would work on this. It'd be great to fix one aspect of bug filing (such as the use of keywords), but it'd be even better to take a look at the entire workflow and figure out ways to make it suck less.
For example, the high-level categorizations are pretty awful currently. What is and is not a product is inconsistent and confusing. I started some notes about this at https://www.mediawiki.org/wiki/Requests_for_comment/Bugzilla_taxonomy.
Perhaps that page could/should be re-titled to "Bugzilla workflow" and we could address a few outstanding Bugzilla workflow problems at once?
Yup, I would like that. The workflow does need work, as well as other aspects. I wouldn't want this to become such a major project that it never gets done, but I think there's value in a holistic look at things.
Rob
On Tue, 28 Aug 2012 15:48:51 -0700, MZMcBride z@mzmcbride.com wrote:
Jon Robson wrote:
Personally I don't like these keywords in bugzilla __at all__ and don't use them. This is mostly because I think they are extremely hidden in the interface.
From my point of view bugzilla should provide a status: 'in review' alongside 'new', 'unconfirmed', 'resolved' and 'assigned'
It'd be nice if the statuses weren't all shouted and single words. INREVIEW is pretty unpleasant.
HACKS!
...selector... { text-transform: lowercase; }
Time to start tweaking the css.
This would be useful for cases where a pull request has been sent or a patch attached - so effectively the ticket has almost been resolved pending this last review step.
Right. We've been hitting this more and more lately. It used to be that you'd commit to SVN, mark the bug as fixed, and then if your revision was reverted, the bug would be reopened by the person doing the reverting. With Git, the development workflow has changed, so it makes sense to adjust Bugzilla's workflow as necessary.
I find the status filter much more useful and would allow me to easily see what bugs needs review. It also helps me filter the bug list to attempt to solve things that haven't been tackled.
You should be able to just as easily filter by keyword, but this isn't to suggest that I disagree with your broader points (more below).
If I review a patch or a pull request and don't think it's suitable then I would propose that we mark it as REOPENED. This at least signals to the provider of the patch that more work needs to be done.
Thoughts?
I think we should take a holistic approach to the Bugzilla workflow. I was hoping the incoming Wikimedia Foundation entomologist would work on this. It'd be great to fix one aspect of bug filing (such as the use of keywords), but it'd be even better to take a look at the entire workflow and figure out ways to make it suck less.
For example, the high-level categorizations are pretty awful currently. What is and is not a product is inconsistent and confusing. I started some notes about this at https://www.mediawiki.org/wiki/Requests_for_comment/Bugzilla_taxonomy.
Perhaps that page could/should be re-titled to "Bugzilla workflow" and we could address a few outstanding Bugzilla workflow problems at once?
MZMcBride
wikitech-l@lists.wikimedia.org