Closed Bug 120233 Opened 23 years ago Closed 22 years ago

Discussion Bug - Bug Overload

Categories

(Bugzilla :: Bugzilla-General, defect, P1)

Product:
Bugzilla
Component:
Bugzilla-General
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: CodeMachine, Assigned: justdave)

Details

We have way too many bugs.  Not serious life-altering show-stopping bugs, but
little niggling bugs that you might not notice were there at first.

Release after release goes by and more important things always prevent them from
being fixed.  This isn't good enough.

Ideally we want to get to the stage where each release is "no known bugs".  2.16
was intended to be this, but it ain't gonna happen.

This is a brainstorming bug, no idea is too ridiculous - it could lead onto a
useful idea.
It seems that in the rush to get in new must-have features, the problem is
getting worse, although I doubt our bug density is increasing.

Can we just set this as a release blocking goal for a release, and clear off all
other goals, or is this impractical?

Do we instead need to do this over several releases?  Perhaps we could set a
firm release blocking goal of a certain decreasing number of bugs each release?
The trick here is to take one CGI at a time. For example, as I templatise, I'm
also taint-moding, and trying to fix other little bugs that have been open for a
while. We should start doing that thing where each bug has the relevant filename
stuck in the status whiteboard again; then people can query for one filename and
make an uber-patch with lots of little niggles sorted.

The number of little bugs will go down, and the ease of fixing them go up, as
the code gets cleaner. Templatisation helps a lot here.

Gerv
>The trick here is to take one CGI at a time. For example, as I templatise, I'm
>also taint-moding, and trying to fix other little bugs that have been open for a
>while.

I've been doing that too, but I'm starting to think it creates more problems
than it solves.  After all, the more problems you fix the bigger your patch and
the harder it is to test and review.  We need to be careful about how many
changes we roll up into uber-patches, which basically just means use common
sense.  If I could have done it over again, for example, I probably would not
have rewritten the HTML for buglist.cgi in the process of templatization but
rather split that into a separate bug and handled that once the templatization
work was completed.


Myk - templatisation spanks a file so hard that it ends up being mostly a
rewrite anyway. So, IMO, the reviewing burden is not raised by fixing other bugs
at the same time. Certainly, review-of-templatisation +
review-of-bug-fix-patches > review-of-templatised-fixed-patch.

Gerv
I think the combination of these two aspects would be best:

- As Gerv said: When you are working in one area, watch out for other bugs in
the same area that you can easily fix on the way. The benefit here is that you
avoid the cost of multiple "context switches" in the developer's mind.

- But at the same time keep separate issues separate. If all the issues are
one-liners, this can be as easy as providing a patch that has a handfull hunks,
each being a fix for a different problem, and an explanation that the first hunk
is for bug x, the second for problem y, etc. If the issues are more complex,
some other mechanism is needed. 

I have some good experience with fine-grained, incremental development: working
in a cvs tree, checking in very often. Of course, this only works because I do
not need to get someone to review my changes before checking them in. This way,
you can separate huge, mostly mechanical changes, like moving code from one
place to another, replacing something with something else in many places, or
adding a lot of explanatory comments, from significant semantic changes. At the
same time, it gives you a changelog with the history of the changes / the patch.

I believe that this will make life easier for reviewers, too: In addition to the
big all-inclusive patch and the resulting file(s), they have the sequence of
steps that lead to the patch, together with an explanation for each step. If a
reviewer trusts the patch author to work carefully, he can just glance over the
mechanical changes, or even rubber-stamp them, and concentrate on the
significant parts.

Now, if this sounds a bit like BugzillaPlus, then you are right. See bug 104681.
I think that the recent templatization patches would be easier to handle if they
had been created this way. (But of course, this is pure speculation :-)

> So, IMO, the reviewing burden is not raised by fixing other bugs
> at the same time. Certainly, review-of-templatisation +
> review-of-bug-fix-patches > review-of-templatised-fixed-patch.

To clarify: I disagree with this, if the fixes are not clearly separated from
the rest of the changes.
For this "fix many bugs in a certain area together" to work one needs proper
code modularization (partly already done with the one-file-per-module approach)
as well as a correspondence between bugs and code areas / modules. The latter is
partly solved by the use of components, but some components are very large so
that the use of tracking bugs (or having the filename and/or code area in the
summary) or (not-yet-existing) sub-components may help. If the bugs were clearly
associated with a finite set of features, it would be possible to formulate
goals for each feature for a release (prototype/alpha, usable/beta,
bugfree/gamma or some other levels), and easily identify the bugs associated
with each level. Imagine a list of already bugfree features growing with each
release. BTW, mozilla has the same problem. See FeatureZilla bug 75172.

I disagree with this. Whilst multiple-_small_-bugs-at-once (emphasis on one
file, < 5 lines) may be easier to review in some cases, tehy cause problems when
trying to trace down regressions/history later on. checkin comments mentionging
5-10 bugs are a real pain to work with.

I think our real problem is that we need more reviewers. The current default is
2 reviewers unless someone thinks otherwise. I'd like to change that to 1
reviewer unless that reviewer/submitter wants more eyes. I know that multiple
reviews find issues, but I think that unless we have some way of tripling the
number of reviewers, its hurting us more than it helps.
We had the same problem when we had single reviews.

We introduced double reviews because we had extremely serious bugs get through
review for both 2.12 and 2.14.  We need to at least give it a chance and see
what happens for 2.16 and 2.18 I think.
I think uber-patches are not so good an idea, for reasons already explained
(just adding a vote. :-)  But there is 1 other reason not to do it.  Some of us
would like to help more, but our time is very limited.  If the bugs are all
large, it's hard for us to help.  And, if people are doing uber-patches (or
multi-patches), then any work I do on the small patches are wasted because the
multi-patch already contains my work, and it isn't always clear the work is
being done when it's rolled up like that.  It won't take too many of those
conflicts before I stop contributing.

I would really like to see b.m.o change the goal for the next release to NOT add
any more major items, but just take/add/use all the existing bugs with patches
to reduce the count.  OK, probably have to do any security needs that come up,
but those are the only new ones allowed!
Since this is a discussion bug, and no idea is too ridiculous, let me describe
another idea. This new idea depends on a checkpoint- or milestone- or
developer-release-system for bugzilla as discussed in bug 120236, so that nobody
will ever have to check out bugzilla code for a production system using a
timestamp any more. In this new world, it may even make sense to create branches
for each of the "milestones", so that some critical fixes can be checked in on
such a branch in case the next checkpoint is still some time ahead when the bug
is discovered.

Assumptions:
- There seems to be consensus that big patches for multiple issues are evil. 
- Reviewers are a scarce resource.

Idea:
- Have many small patches/checkins, and accumulate them for review afterwards.

Of course, this needs some coordination to work. First, for small and obvious
fixes, the current method of creating a patch and having it reviewed before
checkin is preferred. But for larger projects which are local to one or two
files, it may make sense to officially start a project by creating a bug for it,
getting permission from justdave to work on it (e.g. if something is a release
blocker, it will have to be done anyway), then checkin incrementally as you work
on it (referencing the project bug and rephrasing the changes in the log
message), and when you are done with it, post a link to a bonsai query for
checkins referencing your project bug for reviewers. Then, with the "show me all
the diffs" button, reviewers can follow your work chronologically which should
make review a lot easier. 
At some point, justdave stops giving permissions for new projects, and all
existing projects are expected to be finished within a week. Then, those that
still need review are examined, and either some reviewers are found who pick
some nits and help get the project finished, or if noone is able to review,
justdave can decide to tolerate it (either just for the current checkpoint, or
completely), or, if there is no hope, it can be backed out (i.e. the file(s) are
reverted to the original versions, and the (hopefully small) number of other
fixes that went into that file since then is reapplied. In case of a backout,
the work that has been done so far is attached to the project bug as a single
patch, for future reference.

In this model with some "stabilization period" and "freezes", huge, global
changes could get in as "carpools" at the beginning of a cycle (e.g. the
namespace introduction).
Actually, doing fine grained development like this in the main cvs tree seems to
have disadvantages: The revision history will accumulate a lot of unnecessary
entries that make it harder for future developers to scan through it and find
the relevant information quickly. So maybe it's better to do this kind of
fine-grained checkin + review in a different tree, and then port it into the
main tree as a whole. See this article, e.g.:
http://www.lib.uaa.alaska.edu/linux-kernel/archive/2002-Week-04/1178.html

Bradley, Kevin: do you want to respond to this?
I prefer the fine grained stuff to the "6 things all in one go" type commit. The
problem on lkml is that of people commiting works-in-progress. Thats not an
issue for bugzilla - stuff checked in has undergone review, or its on a branch.
Well, in comment 11 I'm describing the possibility of fine-grained,
work-in-progress checkins with accumulated review afterwards. But maybe that
could be better done on a branch then? But then the checkin on the trunk would
still be 6-issues-in-one, or when moving from the branch to the trunk the patch
would have to be split again into some reasonable pieces.
- Fine grained stuff is _much_ better to review. Why do you think we take ages
to review templatization bugs? And undoubtedly review is done better on shorter
bugs; my eyes gloss over 300+ line patches.

- The problems discussed in this bug involve lack of people to work on bugs,
along with  , perhaps, some work needed on setting priorities/severities and
observing that when fixing bugs.

- Apart from fixing priorities and freeing people up with faster reviews, we
simply need more developers if we want to set harder targets. Otherwise, I think
we are fine as we are, speedwise.
Follow up to comment 12...

Andreas, you have point, but IMO, it's outweighed by the way development is
done, i.e. by lots of people each without a lot of time (except for those like
Myk who do this full time).  To rip a paragraph out of the article you posted:

---
Lots of small checkins and branching means a lot of history which is
useful to me but to nobody else. ...[snip]... It does not matter if I
backtracked and discarded some twigs to get to that final end point,
***the rest of the world only cares about the end point.***
---
Please note that last line that I put *'s around.  I'm sure the author didn't
mean it quite the way I'm going to say this but...  I really doubt that many
people are going to try to scan thru the history.  I would think most people are
going to pick a point in time, grab the code there, and off they go (or at least
that's the way I operate. :-)  The many details are there for those that want to
look, but what really matters is getting the changes in, and I think that's only
going to happen if the vast majority of the patches are small (I know that's
relative :-) ; and if that creates a lot of history, so be it, at least the code
got written.  (and we all want the code written, right? :-)
if that's the case then all development should be on branches and we should 
just land the branch for each release.

i like being able to follow the trunk and understand what's going on.  I like 
being able to find out which person really made which change.  And I don't like 
trying to figure out branches. However my experience is mostly w/ NSPR, JSEng 
and JSDom branches where some of the changes might not even have been done in 
cvs.mozilla.org
> - Fine grained stuff is _much_ better to review. Why do you think we take ages
> to review templatization bugs?

Imagine how much longer it would have taken if we'd done it incrementally. We'd
be here until Christmas.

About branches, I agree with timeless - the trunk should be the main
development, and we should use branches only in rare circumstances.

Gerv


> > - Fine grained stuff is _much_ better to review. Why do you think we take ages
> > to review templatization bugs?
>
> Imagine how much longer it would have taken if we'd done it incrementally. We'd
> be here until Christmas.

Oh, sorry, that's not what I intended. Templatization bugs are _inherently hard
to review_ because usually all lines are touched, and reading the diffs is hard.

I just wanted to show that when the patches are very large, review is slower and
harder. If we can avoid making large patches, we should (so not compounding
fixes is good); however, some changes simply whack whole files.
> Oh, sorry, that's not what I intended. Templatization bugs are _inherently hard
> to review_ because usually all lines are touched, and reading the diffs is hard.

No wonder you've been having problems if you've been reading the diffs!

Review the whole new file as a unit, perhaps side by side with a copy of the old
one. It's much easier.

Gerv



Hm. If I understand you correctly, we have the following axioms:

(1) You (or at least some people) want (or prefer) fine-grained development, 
    so that a single checkin fixes a single issue.
(2) You (or at least some people) do not want work to happen on branches (with
    only very few exceptions).
(3) There is a significant time interval between a request for review and the
    completion of the review (at least in most cases).
(4) All code must be reviewed before it is checked in on the main trunk.

And now take a look at cases where several changes have to be made which are not
independent of each other, at least patch-wise. (I claim that this case is quite
common. E.g. if you rename some variables, add some comments, refactor some
code, rewrite some functions, make an optimization here, a simplification there,
change the api a bit...)

In this case, you either (a) end up posting an endless series of big attachments
to bugzilla (like Myk's 100KB patches in bug 103778), or (b) your development is
blocked for extended periods because you are waiting for review for your first
step(s), and check them in, and then continue with your next steps.

Neither of these outcomes is good: 

In case (a) it is hard to deal with these large patch sizes over and over again,
and you don't have the history (the logical sequence of changes /
transformations), you can't say "Ok, I have reviewed the first half of the
changes, this shouldn't break anything, so you can check them in, and I'll
continue reviewing the second half next week" because this patch is
all-or-nothing. And the longer it takes, the more likely it is that it conflicts
with other changes that are happening in parallel. 

In case (b) bugzilla development will simply be stalled, which is even worse
since there's a lot of changes to be expected in the near future:
modularization, mod_perl support, group system rewrite, move to generic fields,
...

Please correct me if this reasoning is flawed.

I think (3) is the hardest of the axioms, we need to accept that.
So, given (1), the only way out is to give up (2), or to give up (3) and allow
review-after-checkin a la comment 11. I don't see Dave making compromises with
(3), so my suggestion would be to give up (2), either in the form of a separate
cvs repository a la BugzillaPlus, or in mostly unrestricted branches in the main
cvs repository.

Or do we accept the consequences (a) or (b)? 
Or aren't they necessarily consequences?

Comments?
In my opinion, big patches are always going to be hard to land. Sure a branch
would let them land faster, but it doesn't avoid the merge problems, and I don't
think that's worse than blocking work till it lands.

I also think that once templates have landed, a lot of blocked work will move on
better, and we will feel less blocked. 2.16 was a lot of hard work. Things will
improve.

Gerv: I do that, but sometimes I don't have a tree to apply the diff to :) And
it's still a tough review.
I'm a consultant who is currently getting a Bugzilla installation up and 
running at this customer's site.  The site is securely tugged away behind a 
firewall, so CVS access is difficult.

When I first fetched Bugzilla, I knew not what I know now.  Accessing CVS at 
home and carrying stuff to the customer's site did not seem worth the trouble.  
So 2.14.1 is what I've got and have been customizing these days.

I try to give back to the free software community.  Your page 
http://www.mozilla.org/projects/bugzilla/how_to_help.html talks to my heart.  I 
diligently file bugs as I find them.  However, in the Bugzilla case, that's 
been somewhat of a frustrating experience.

Basically, 2.14.1 works.  That said, there indeed are many little nagging 
bugs.  Yet time after time after time, as I file a bug, I find soon receive an 
email that it has resolved as a duplicate, as it is a special case of some much 
more general "grand scheme" thing.  Typically that grand scheme is something of 
the category "I'd just love to have that".

I am willing and think myself capable to come up with a patch every once in a 
while, certainly for many of those little nagging creatures.  But my general 
impression, contrary to what you say in that page I've mentioned: Patches 
against the last tarball revision 2.14.1 are close to useless.

I fancy there must be more people out there with tiny, not so terribly mission-
critical installations of Bugzilla.  These people could ideally help you very 
well with those tiny, not so terribly mission-critical bugs.  If only you'd let 
them.

To let us, give us 2.15.x "bleeding edge" tarballs regularily.

You have a shortage of reviewers?  Put stuff to be reviewed in there, we'll do 
the reviewing once we get to see it.

I encourage you to run your production site as 2.14.1, while some 2.15.9 or so 
is out on the "bleeding edge" download page.  Don't do it the other way round.

(As a side remark: You folks have all read 
http://www.tuxedo.org/~esr/writings/cathedral-bazaar/cathedral-bazaar/ , right?)

Andreas: believe it or not, that's actually the plan as soon as 2.16 is out.  We
didn't start with the 2.15.x right now because the Templatization rewrite was
gutting major portions of Bugzilla and it would just outright confuse people to
have big chunks of the hierarchy get renamed and moved around and such between
releases not just once, but several times (causing them to have to redo any
customization they've done over and over).  There will be 2.17.x releases. 
We'll be aiming for once a month.  People reporting bugs in 2.14.1 that were
fixed a long time ago in CVS (but not released yet) is another one of the
primary reasons for doing it this way :-)  It'll just save work for everyone.
Dave: Seems to be my fate here: One more dupe.  One more time, I've suggested 
something, only to find out it is already in the doing, and, of course, again, 
I'd just love it if it were here already :-) .

Good news, though, thank you!  Some more tunnel ahead of us, and, as it 
happens, nobody quite knows how much tunnel (at least 
http://www.mozilla.org/projects/bugzilla/status_updates/ doesn't), but there's 
light promised!
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.18
I think we should close this bug; the mailing list and newsgroups are more
appropriate forums for discussion.

Gerv
Resolving.

Gerv
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → WORKSFORME
Target Milestone: Bugzilla 2.18 → ---
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.