Project is under heavy load

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

Hi all,

I have been tied up a lot these days, the project is under a lot of
load, and as a result the pressure on myself and individual
contributors alike is mounting, which is not helping, so lets try to
alleviate some of this pressure.

My goal with this email is to improve the efficiency of this project,
in terms of safely landing new features.


Debunking the magic bullet fallacy
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A lot of people around here want to land a lot of new *features*, and I
want to be very clear that we are not talking about patches, we are
talking about features, most of which have a great impact on core
architecture and design.

Because of a number of circumstances, not least of which is an increase
an overall load, review cycles are becoming less efficient.

This has led some people to believe that:

  a.) We dont have enough people with the power to review patches.

      I agree with this, which is why I have delegated a lot of feature
      branch reviews to Jürg, while only discussing things between us
      at a high level in some (not all) circumstances.

      I want to find another lieutenant who can review feature
      additions, but for this to happen people need to understand the
      codebase as a whole, care about how the codebase evolves as a
      whole, and beyond this they have to earn my trust such that I am
      assured we are united in a common vision.

      Unfortunately this takes time, and I find myself lacking in
      suitable candidates, but believe me that I am hungry to find one.

  b.) A core developer should be able to land patches without review,
      and fix later if a problem arises.

      In my experience I have often worked this way, and we currently
      do have a short list of committers in BuildStream. It works well
      as long as said core developers are personally responsible and
      accountable for the patches they are sure about.

      For a contributor to retain committer status, I have to know that
      the "fix it later" part has an immediate turnaround and takes
      precedence over all other ongoing work, otherwise this is a
      violation of trust, and it means that "being sure about the
      commit" is in fact a lie.

      Also, this cannot mean unreviewed landing of feature additions,
      which necessarily impact API stable interfaces or make
      significant changes to core architecture and design, which may
      not need detailed code review, but needs discussion an approval.

      We are not suffering from an influx of easily reviewable bug
      fixes which are not landing, which happens sometimes in larger
      and more mature software projects: We are suffering from an
      influx of many large and complicated feature branches that all
      want to land now, so it should be noted that having more
      committers in this scenario does not fix the current problem.

  c.) The bar is too high for landing patches.

      This is just patently false. The last two weeks I've spent fixing
      regressions, and untangling structural weakness in our codebase
      is proof enough to me that the bar needs to be raised, not
      lowered.


In summary here, there is no magic bullet which will allow your
features to land faster, even if we had more core maintainers, large
core features could not safely land any faster, and relaxing the review
process here can only result in a complete disaster.


What can we do to make the project more efficient ?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Asides from the fact that you just cannot land a lot of features at the
same time safely, there are a few things we can do to greatly improve
the efficiency in how we land features.

On my part, I'm going to try to do a better job in roadmapping, I had
this working much better last year but in recent times I've been too
tied up with specific goals to paint a better picture here.

In order for Jürg and I to handle review of your features more
efficiently, I'm going to lay out a list of things you can do to make
all of our lives easier.


  Round tripping is more expensive than actual review
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  The life of a maintainer is wrought with micro interruptions and
  context switches, as such it is important to state all of the facts
  at once and avoid round tripping.

  Unless it is clearly in context in a mailing list thread or issue
  report thread: Never assume we know what you are talking about, state
  all of the facts and provide links.

  When writing an email to the list, or a comment on an issue report,
  provide _all_ of the details at once. Dont attempt to entice me into
  a long discourse, instead strive to provide a clearly thought out
  problem statement with the goal of having it solvable in a single
  reply.

  You may find that half of the time, going through the exercise of
  clearly stating a problem in an email results in your having
  understood the problem, and sending the email becomes unnecessary
  (while the activity of writing it was valuable).


  Only submit patches that you believe will land
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  We've been very relaxed in the last year about how we do reviews,
  patch review has been a place for mentoring and discussion about
  how to land the patch.

  Remove this from our workflow by only submitting patches for review
  that you are personally near certain will land.

  This is impossible for a first time submitter, and we understand
  this. Don't be shy to submit your first patch, and if you have team
  mates who regularly submit patches to BuildStream, get them to review
  it first before submitting it for review in our tracking system.

  I guarantee that the patches you think are ready to land, will
  most often not be ready. But I also guarantee that this will
  *greatly* reduce the round tripping which is bogging us down and
  clogging up the review pipeline.


  Try to leave the code in a better state than how you found it
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In many cases, there are multiple ways to implement a feature,
  usually these consist of a quick and dirty way, and a way which makes
  sense.

  Choose the way which makes sense, even though it almost certainly
  costs a bit more legwork in the short term.

  The quick and dirty fix is almost always going to be percieved as a
  threat to the longevity of the project at large, and it is our
  position as maintainers to defend the project against such threats.

  Dont make us feel threatened by your patches, instead try to share
  in the responsibility of making the whole codebase better all the
  time.


  Try your best to suggest a solution, or more than one solution
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  When you have a feature to implement and you see the current
  architecture does not "fit" with what you intend to achieve,
  this means that we need to reconsider the architectural design
  to afford for the feature you want to bring to our users.

  In these cases, try to be creative and think of a solution which
  best fits the architectural design of the current codebase, and
  propose that solution.

  In other words, try to make it easier for the core maintainers to do
  their jobs: it's much easier for us to look at a proposed solution
  and identify problems with that, and possibly point out problems with
  the proposal for you to work on, than to do legwork and come up with
  a plan ourselves to solve the problem you want to have solved.


  Solve the mentoring problem outside of the maintainership
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  Last year I spent a great deal of time coaching younger developers in
  the ways of writing maintainable software.

  I actually love teaching, so I am particularly at risk of falling
  prey to an invitation to mentor you on how to write your patch.

  In the present climate however, it is unreasonable for me to be
  spending my time in such activities - when I happen to find time, I
  will indulge myself.

  If you have juniors on your team, then have your senior developers
  mentor them as they develop features for BuildStream, help them write
  proposals to the mailing list before hand to ensure that we are first
  in agreement on landing the feature and that the architectural
  changes involved are going to be accepted.

  Mentor them to be good at submitting changes to FOSS projects.


  Respect the style in place and be consistent
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  It is of great importance that the codebase remain consistent such
  that it can be read and understood as a whole to any newcomers.
  Linters are not sufficient to address this.

  Not all of our coding practices are the best, if you do not agree
  with a practice, propose that we change it, and if we agree to change
  it, we will expect a patch which changes the practice *unilaterally*
  across the codebase.

  It is not acceptable to leave the codebase in a state where things
  are done in one way here, and in another way there.

  This should really go without saying: Don't let your personal taste
  in coding style get in the way of having a consistent code base, this
  can only be a constant source of friction in the review process which
  will waste time and leave everyone dissatisfied.


Summary
~~~~~~~
In closing, there is a lot we can do to improve the flow of reviews for
the many features that all want to land - increasing the volume and
pressuring for more reviews more often from the maintainership is not
going to magically make the feature branches safe to land sooner.

Let's be more professional and productive, share the responsibility
more evenly, and while we wont have any magic bullet, we will certainly
improve the situation.

And by all means, lets reserve some measure of time for horsing around,
poking fun at one another, and laughing a bit from time to time.

Keep it light hearted and fun, and we'll all have a great time while
accomplishing great things.

Cheers,
    -Tristan