[OPEN-ILS-DEV] Mr. Rylander, we have a problem.

[Mike Rylander]

Jason,

First, thank you for taking the time to pen a thoughtful message.  As you
might imagine, I see things from a different perspective and don't
necessarily agree with you, and while I may not change your mind I believe
we can understand one another.  About one thing we certainly can agree --
you, personally, take a very balanced approach to code review and feature
testing.  I appreciate that for myself and for the community, and I ask
that you accept my apology for the curtness and flip attitude with which I
addressed you in IRC yesterday.  The tone I used with you was unwarranted,
and I regret directing my frustration at you personally.

However...

On Thu, Mar 14, 2013 at 9:11 AM, Jason Stephenson <jstephenson at mvlc.org>wrote:

> Mike,
>
> You don't pay enough attention when someone is trying to tell you
> something.
>
>
I agree that, in the heat of the moment yesterday, I did not listen to you,
specifically, as closely as you deserved.  I apologize for that, and offer
only the reason, which is not an excuse, that after constant and consistent
negative communication from one other person I let my emotions boil over.
 That was not your fault, and I should not have taken it out on you.


> Yesterday, I was very angry about Launchpad bug 1154766
> (https://bugs.launchpad.net/**bugs/1154766<https://bugs.launchpad.net/bugs/1154766>).
>  I did not appreciate that
> the first email I received from Launchpad had a "Fix Committed"
> status.  I objected to this fact rather vociferously on the bug and in
> IRC.  My point was that no one outside of ESI had a chance to even
> look at this new feature before it had gone into master.
>
> You took this as a complaint that *I* had not had a chance to look at
> it.  You seemed to imply that this was a contest about who had more
> commits, or something.
>
>
If that's how I came across, I'm sorry.  I took the complaint as you
intended it -- that ESI, as an entity, did not give anyone notice before
committing a new feature.  And therein lies part of the problem, the
mistaken idea that Lebbeous, or any other employee of ESI, is not an
individual when it comes to the exercise of the commit bit or any other
interaction within the community.  I'm sure that you want to be seen as an
individual as well.

While your point that very few people are reviewing other people's
> commits and that many branches have sat for months with no apparent
> action on them is well taken.  That is, however, beside the point of
> my argument.
>
> It was agreed at the January developers' meeting that new features
> should wait a week before going into master.  This was in order to
> give others in the community a chance to look at, to ask questions
> about, and to comment on the new functionality or its implementation.
>
>
That is not what was actually agreed to, and, in fact, what you state here
is a much softer, watered-down version of what was originally proposed.  At
the risk of dragging things off-topic, I feel this is an important point to
discuss.  It's the main point upon which your contention rests.  Here is
the relevant IRC log section:

14:16:28 <gmcharlt> #topic proposals changing the procedure for  reviewing
and pushing new feature patches
14:16:53 <gmcharlt> #info proposal #1 from tsbere = To encourage more
community communication and involvement I propose that new features (not
bugfixes) require sign-offs from two or more groups in the community before
being committed, regardless of commit bit status. That would prevent any
single entity from pushing new features in wi
14:17:06 <gmcharlt> #info ...  without the community looking over them just
because they have one or more core committers in their employ.
14:17:28 <gmcharlt> #info proposal #2 from tsbere - On a similar vein, to
ensure communication time with the community is possible I propose that new
features need to be in a pullrequest state on Launchpad for a minimum of a
week before a core committer pushes them into the system. Again, not
bugfixes.
14:17:57 <kivilahtio1> Sounds great
14:18:18 <gmcharlt> FYI, I will be ending the meeting no later than 12
minutes from now
14:18:42 --> Callender (~
quassel at 50-79-238-214-static.hfc.comcastbusiness.net) has joined #evergreen
14:18:56 <bshum> I'm +1 to proposal 2 if only because I've been participant
to pushing new changes without waiting for all the comments to settle first.
14:19:09 <gmcharlt> tsbere: your proposal text seem pretty complete, but is
there anything you want to add?
14:19:34 <tsbere> I don't think so, unless people have questions for me,
anyway.
14:19:42 <jeff> seems reasonable at first glance. i'd be interested in
hearing from anyone who sees issues or overlooked subtleties.
14:19:48 <paxed> are there enough active community members so proposal #1
doesn't keep stuff in limbo?
14:19:55 <senator> i'm -1 on these
14:20:07 <senator> because of stuff sitting in limbo as it is
14:20:28 <senator> in the future, when we have more active testers of
master and feature branches, maybe
14:20:37 <kivilahtio1> that is a solid point
14:21:01 <kivilahtio1> well, we need to wait for 1 week for acceptance. If
things stay in the limbo for two weeks, we can ommit to master?
14:21:06 <tsbere> I figure that saying "no, work from group A can't go in
without someone else looking at it" results in each group doing more
looking at other work, instead of possibly looking at only their own stuff.
14:21:37 <kivilahtio1> but this would help alleviate issues with
internationalization for example
14:21:41 <phasefx> I'm -1 on first, +1 on second.  I think if multiple
committers from the same organization are inappropriately fast tracking
things, the community should call them on it, but we shouldn't encumber
ourselves beforehand
14:21:58 -*- jcamins probably doesn't count, being a Kohan, but can't
resist throwing in his two cents on an issue near and dear to his heart.
14:22:00 <kivilahtio1> so we have more time to make sure that things can be
internationalzied
14:22:13 <jeff> trial period, where it is encouraged to wait the week and
get sign-off from another organization, and see if this results in more
things stuck in limbo?
14:22:41 <jcamins> Koha has adopted a QA policy requiring at least two, and
generally three, different entities to review a patch before it gets pushed.
14:22:44 -*- denials points at
http://evergreen-ils.org/dokuwiki/doku.php?id=dev:signoff_review_checklistagain
(which we liked last time, but hasn't been formally adopted) for the
sake of kivilahtio1's i18n concern, at least
14:22:59 <dbwells> I think both of these proposals are good common sense
guidelines, but I am not in favor of mandating them until there is a clear
reason to do so.
14:23:11 <jcamins> It works well enough for keeping problematic code out,
but it does not do much to encourage people to review code.
14:23:29 <denials> keeping problematic code out is a win :)
14:23:37 <denials> my code would never get in
14:23:43 <jeff> one issue i can see would be where there is not
cross-organizational expertise in an area. no examples immediately jump to
mind.
14:23:54 <berick> edi comes to mind
14:24:32 <jeff> apologies if i'm forgetting/overlooking a serials expert,
but senator or dbwells being on vacation could present a potential
roadblock.
14:24:33 <kivilahtio1> don't we get karma for code reviews?
14:24:34 <kmlussier> berick: I agree with you on the edi point.
14:25:08 <phasefx> for documentation commits, I'd be inclined to reduce
barriers even further, and not even require double sign-offs
14:25:14 <denials> yeah. I'm with dbwells. Maybe add a note to the commit
checklist saying "Generally it's a good idea to let the community take a
peek before this is committed - has the bug been open and branch visible
for a few days?"
14:25:23 <paxed> and what jeff said tells me there's not enough active
people for either of those proposals to work for now...
14:25:25 <denials> phasefx: we don't require double sign off for docs. so
that's good
14:25:31 <phasefx> gut
14:25:33 -*- gmcharlt tosses out an idea -- since part of the concern is
getting eyes on code and making sure that more than one organization
understands what's going on ... what about trying the occassional 30-minute
IRC meeting where somebody does a walkthrough of a new proposed feature
patch?
14:25:38 <jeff> tsbere: do you have any opinion on adopting these as
recommendations and not strict rules?
14:26:04 <paxed> gmcharlt++ # would also help noobs (like me) understand
what's going on
14:26:04 <tsbere> I already use them as personal guidelines, I figured
making them official might help ;)
14:26:11 <kivilahtio1> gmcharlt: dont we have blueprints for that?
14:26:15 <paxed> ...without explicitly bugging people in here
14:26:24 <gmcharlt> kivilahtio1: blueprints aren't interactive
14:26:28 <jcamins> Since you have more people who can push code, a
compromise position might be to send messages to the developer list saying
"I would like to push these five patches in the next two days. Would anyone
else like to test?"
14:26:43 <jeff> "new feature Q/A" in irc sounds promising.
14:26:53 -*- senator can live with what jcamins is saying now
14:26:55 <kivilahtio1> jeff: ++
14:26:56 <senator> we sometimes do that anyway
14:27:19 <senator> and gmcharlt's idea doesn't sound bad either
14:27:39 <jcamins> senator: yeah, I've seen some announcements on IRC, and
it seems a fairly simply addition to put the announcements on a mailing
list.
14:27:49 -*- jcamins is going to steal gmcharlt's idea for Koha.
14:27:58 <kivilahtio1> we just need to read the mailing list
14:28:41 -*- gmcharlt calls a vote - that 1. tsbere's two proposals be
accepted as recommendations but not requirements 2. and that around the
time of the conference, we see how the pullrequest queue is doing
14:29:36 <denials> 1. +1
14:29:45 <bshum> +1
14:29:57 <Dyrcona> (+1, +1)
14:30:03 <denials> 2. Ideally before, so things could get lined up for
$RM_VERSION_NAME
14:30:11 <kivilahtio1> (+1,+1)
14:30:18 -*- denials takes off
14:30:58 --> swills_ (~swills at 66.186.120.60) has joined #evergreen
14:31:46 <jeff> +1
14:31:54 <paxed> +1
14:32:22 <phasefx> +1
14:32:51 <senator> +1, +1
14:33:48 <dbwells> +1, +1

First, it was accepted as a recommendation for the short term, and one to
revisit at the conference or thereabouts.  Indeed, some of the most
level-headed and conciliatory community members and developers spoke up
specifically to weaken the proposals both to recommendation status and as
to the content.

Second, the proposal deliberately conflated "developer" with "employing
organization".  There are many reasons this is bad, some stated during the
discussion and some not, but very few that are at least conceptually good.
 But, primarily, this is not how the Evergreen community has worked
together in the past.  Every individual has always been separate in the
eyes of the community.  I've been called out by Bill, Jason and Lebbeous,
publicly as well as privately, on issues relating to code.  Because this
organizational grouping is not the "Evergreen way," I will not act under
such an interpretation of any recommendation.

To summarize, as you see, there was significant objection to both the
spirit and letter of the proposal.  Simply saying it was accepted is
missing important nuance.  jcamins noted that, for the Koha community, the
goal of having multiple organizations review a feature is not served by
this process of delay.  History in our community corroborates this.  This
was the sense that kivilahtio1 and senator had, that things would still sit
in limbo.  phasefx objected to the idea that we developers are not
individuals, and as many noted, there are large areas of the code where the
organization-grouping would significantly restrict the ability of code to
be reviewed and committed.



> You seem to be under the mistaken apprehension that no one in the
> community outside of Equinox is looking at Launchpad.


Indeed not.  The email feed certainly suggests otherwise, and I have not
yet argued against the evidence.


>  Thomas and I do
> regularly look at code posted via Launchpad.  In the vast majority of
> cases, we have no opinion on the code or don't feel competent to judge
> the code on its merits.  (The latter is most often true with
> acquisitions and serials since our consortium does not use these
> features.)  I do, however, regularly update my development branches
> with branches posted from Launchpad.  When I have the chance to test
> something to my satisfaction, I will sign off and commit it.  I also
> load acquisitions and serials branches on this server for Kathy
> Lussier to review.  In fact, she was looking at all of the
> acquisitions improvements branches that went into master yesterday,
> plus a couple of others.  Perhaps we are not fast enough for you, but
> we are looking at this work.  We do have other responsibilities, you
> know.
>
>
Let me state clearly and emphatically, I greatly appreciate your efforts to
facilitate testing of features on your servers.  Thank you!

When this has been discussed in IRC before, as I'm sure you recall, I have
maintained the opinion that broad integration branches, given today's
number of incoming features and the speed of development by the community
at large, are a distraction and waste of resources.  I believe that if a
feature branch can be tested in isolation, and passes those tests, it
should go into master.  An integration branch that does not have every
pending topic branch merged will not be able to confirm adverse interaction
between proposed features.  Using master as the integration branch allows
faster, targeted testing of specific, small additions by a few to confirm
that the feature works in isolation, while a larger set of users can then
reconfirm that the feature works in combination with what has been accepted
into the mainline code.  Integration branches, by contrast, complicate
testing by increasing variables, introduce risk by requiring multiple
merges, increase testing lag as master changes and those must be further
incorporated, and generally decrease the dependability of testing due to
all the foregoing.

The reason I hold this belief is that it I watch it work in practice.  This
serialization of feature testing streamlines the setup and maintenance of
the testing environment so that more features can be address in the same
amount of time, lends itself to dis-integration and distributed testing,
and is parallelizable to a large degree.

On the other hand, and this is something I've seen you encounter well
outside the beta crunch, broad integration branches run the risk of getting
into a state where they cannot be rebased to master, meaning that you have
to start over with the multi-merge.  This is particularly painful if there
are seed data changes in multiple branches, as I'm sure you know. These
sorts of problems are much easier to side-step when master is the
integration branch, and features are tested individually from their topic
branches.

I suspect, because of these difference that I have observed in between
these two approaches, that the speed of setup and the effort required to
keep these broad integration branches in sync with updates to the topic
branches as well as to master may contribute to delays in testing.  Is that
fair, or do you find that this is not the case for you?


You also mistakenly asserted that development is Thomas's primary role
> at MVLC.  It is not.  He spends most of his day resolving internal
> issues for our members and making sure that our servers continue to
> run smoothly.  Development of Evergreen is only a secondary part of
> his job, if even that.  He produces the volume of code that he does
> because he enjoys programming enough to work on Evergreen on his own
> time at home.  You made your assumption and you accused Thomas of not
> helping the community based solely on the commit statistics from the
> last year that you posted to Launchpad yesterday
> (https://bugs.launchpad.net/**evergreen/+bug/1154267/**comments/2<https://bugs.launchpad.net/evergreen/+bug/1154267/comments/2>
> ).
>
>
I made my assumption about Thomas' work-time development based also on the
relative amount of normal working hours that he is usually talking about
his ongoing development in IRC.

But when he writes code is really beside the point, and the number of
commits is immaterial.  The ratio of commits to authorships is, however,
very germane.  Perhaps posting only ratios as data would have made that
point more clear, but the narrative was certainly intended to convey this.

I did not accuse Thomas of not helping the community, I simply pointed out
that he does not, as a rule, merge code from others.  If it's true that one
of the goals of the recommendations he pointed out is to have sign-off from
multiple reviewers, or stated another way, to have committers pushing code
they did not author, then he is not participating to a
degree commensurate with the amount of code of his that is pushed.  If he
had not been the one to bring these recommendations to the table originally
this might not be a point of discussion.

My point with regard to Thomas and his recommendations is only this:
process without participation is often worse than no process at all.

With regard to those recommendations in relation to the community as a
whole, except for one of my 79 commits since last September, which I'll
address further below, I feel I have acted in the best interest of the
entire Evergreen community without exception.  I accept that you may not
agree with that, but I hope you will read further and understand why I feel
that way.


> Looking at these statistics, I think you will have to admit that I am
> probably the most balanced of the committers when it comes to number
> of commits of my own that go in compared to the number of commits of
> others that I push.  I am just about 50/50 in that regard.


Certainly, as I said up front in this email.  I was speaking specifically
to Thomas in that comment.  I don't believe I have suggested that you,
personally, do not try to contribute as fairly as possible in both
directions, perhaps even too much so.  Others do as well -- Ben Shum, Dan
Scott and Lebbeous Fogle-Weekley in particular push as much or more code
than they produce.  But Bill Erickson, Galen Charlton and I are also near
the top of that list.  I don't want to risk leaving anyone deserving of
praise out of any lists, so I'll just say that I am convinced that we are
generally a community of developers that work well together and help one
another to the degree each of us has time and talent.


>  Further,
> if you look at who the authors are of the branches that I commit, the
> vast majority are from Equinox developers.


I'm sorry, but that's selection bias based on the simple fact that Bill,
Lebbeous, Jason and I, along with others that happen to be employed by ESI,
produce a significant amount of code.  However, this again falls into the
trap of thinking of ESI as some sort of monolith.  It's not when it comes
to developer interaction in the community.  We're individuals.


>  I also push more commits
> from non-committers than I have committed of Thomas's code.
>
>
As do many.  Dan, Bill and especially Lebbeous are prodigious in this
regard, but I have to say that the numbers show Ben Shum is the top
committer by this measure.  He also pushes a great deal of Thomas' code.
 That doesn't lessen your contribution, nor inflate that of anyone else;
it's beside the point except with regard to those committers that do not
push others code.


> Thomas and I have an agreement to push as little of each others' code
> as possible precisely to avoid contentious arguments such as this with
> the rest of the community.
>
>
That may be your reason, and I have no reason to doubt it, but it's not the
reason I would request abstention from you, were I to do so.  However, I
would not, because I trust your judgement of both the code and your ability
to review it.

The fact that you're both at MVLC matters not a whit to me. It's my belief
that if you test Thomas' code and it works for you, and you feel competent
to review it from an implementation point of view, it is your right and
responsibility to push the code.


> Furthermore, I cannot count the number of times that we have told our
> members--the people who pay our salaries!--that they cannot have some
> feature they desired because we knew that the community would not like
> it, or would not like the way that our members want us to implement
> it.


You're far from the only person that has to say "no," or, at least, "not
quite like that, but how about this instead."  It's a refrain I know better
than most.  If you find yourself saying the former more often than the
latter fairly often then my recommendation would be that you might want to
take a hard look at the approach taken when designing those features.  I
find that there is almost always a solution to any feature request that
need not run afoul of other interested parties' concerns, up to and
including having the feature disabled and invisible by default.


>  The community is larger than MVLC, or ESI and her customers.  The
> community is larger than C/W MARS, NOBLE, and MassLNC.  The community
> includes Indiana, Georgia PINES, SC LENDS, MELCAT, Bibliomation,
> Laurentian University, Mohawk College, TADL, and a whole host of
> others.


Absolutely.  I would contend that a very significant portion of the
community is made up of libraries that use the software without
encountering a single Evergreen developer (that they're aware of) ever.
 The institutions and entities you list there are only those lucky enough
to be in a position where they can allocate at least some resources to
direct community participation.


>  We do our best to consider everyone's needs as well as we
> understand them before we undertake any development work, and if we
> are unsure we ask, via IRC, Launchpad, and/or email.
>
>
If by "we" you mean both yourself and Thomas, I do not agree with that
statement.  I do believe that you, personally, want to take everyone's
needs into account.


> Hence, the desire for the waiting period.  Many of the above have
> developers either on staff or under contract.  They can look at
> Launchpad and can comment on development.  However, when code is
> pushed and bugs practically created in the Fix Committed status, none
> of them have the opportunity to look, to comment, nor to test the code
> if they so desire.  This, Mr. Rylander, is the reasone for my
> expression of anger in IRC and on Launchpad yesterday.  No one outside
> of Equinox was given so much as a "Hey, we're working on this," before
> the code was committed to master.  This is not a personal contest
> between MVLC and ESI about the number of commits.


Indeed it's not.


>  This is about the
> community, communication and respect for the same.


To reiterate, I apologize to you for the tone of my communication with you
yesterday in IRC.  It was unwarranted and I regret it.


>  ESI's actions with
> this new feature show a lack of regard for the community.
>
>
I disagree, and I will explain why I do below.


> I understand that as 2.4 Release Manager you have some prerogatives in
> this domain.  However, that title does not grant you a dictatorial
> positon,


I would state it another way: it is my position to ensure that the release
I'm charged with has the features and fixes in it that will, first,
eliminate as much user pain as possible and second, increase Evergreen's
capabilities and thereby its adoption, while reducing the risk of bugs to
the degree possible.  If you consider my methods for doing this in 2.4
dictatorial, so be it.  I do not.  However, I will elaborate.

I can understand to a degree the consternation regarding, in particular,
the MARC21 feed patch going in.  That is a feature entirely orthogonal to
anything else in Evergreen, of extraordinarily low risk
and immensely useful for external integration.  Even with the reaction
received I would likely still see that pushed (with one process change on
my end), had we it to do over again, because I believe that the community
is served by pragmatic deviation from recommendations.  With the chance for
a do-over, I would have sent out an email to the dev list describing the
change, the small risk, and the benefit, at least a couple days in advance.
 For a change of that size and risk, I feel that would have been more than
enough.

So, if that one commit is truly the source of your primary complaint, we
have found our problem. I believe the community was served by deviating
from the recommendations where benefit outweighed risk and time was short.

As for the other two short-notice branches, both got comments from Thomas,
and for the one that was commented on with a request (
https://bugs.launchpad.net/evergreen/+bug/1154272 ) for a change, that
change was made.  If the purpose of the recommendations are truly to
solicit feedback then in the case where feedback was provided, at least,
the recommendation was satisfied.

Finally, regarding the ACQ branches, they were tested by multiple
developers and by end users to the degree that the test server those users
were given was stable enough to do so.  They were also well outside the
requested week, and it's been explained to me by Evergreen end users in
many, varied organizations that the features and fixes represented in those
branches are of critical importance to the continued use of Evergreen in
several instances.


> and you should in fact have a greater regard for the
> community as a whole in that position, and not just your company's
> contractual obligations to her customers.
>
>
This has nothing to do with contractual obligations.  Our contracts don’t
guarantee that code we produce will be in any specific release.  We
guarantee that it will be completed by a certain date, and offered for
inclusion in the next release beyond that date.  Of course, while true,
that’s also immaterial.  The branches were merged because it’s unfair not
to merge them when they contain bug or performance improvements that the
community needs.

Many in the community, which is much larger than the list you cite above,
have been begging for action on the issues addressed in the ACQ branches
merged yesterday.  Were it not for actions taken by me as release manager
and by other individual developers to address these branches sitting in
limbo it would be at least another seven months before the critically
needed bug and performance fixes made their way to most users.  Your system
runs master, or a very close approximation thereof, and you can because you
have an in-house developer making you capable of running on the bleeding
edge of development.  So you see all fixes as quickly as you choose to
apply them.  The vast majority of sites, perhaps all other sites, do not
have that luxury, so asking them to wait another another 7+ months for the
next release to get features that are complete and tested now is not fair
to them as community members.

The reaction would likely have been less negative had I sent an email
regarding the binary MARC feed work at some point before it was merged.
 This was work we talked about publicly, and was not sprung without any
word at all, but I recognize that lack of developer notification as a
failing on my part in this situation, and I will commit to more closely
adhering to the spirit of the broader community's recommendations.
 However, beyond that, the goals and deadlines laid out in the
recommendations under discussion here where met, both in spirit and letter,
to the degree that I, as release manager against a beta deadline, judged
prudent and in the best interest of Evergreen.

Finally, I want to apologize to everyone for exploding on the
> Launchpad ticket and in IRC as I did yesterday.  It was unprofessional
> of me and inexcusable.



Jason, I echo this.  You feel strongly, as do I.  I apologize to all that
were subjected to my behavior as well.  It's not fair of me to treat others
unprofessionally.

-- 
Mike Rylander
 | Director of Research and Development
 | Equinox Software, Inc. / Your Library's Guide to Open Source
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker at esilibrary.com
 | web:  http://www.esilibrary.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20130314/8d278bb0/attachment-0001.htm>