[yt-dev] proposed change to development process

[Matthew Turk]

Hi Cameron,

I will leave discussion of the rest of your points to others, as I've had
my say.  But I did want to address one thing:

On Thu, Sep 10, 2015 at 6:29 PM, Cameron Hummels <chummels at gmail.com> wrote:

> As a first point, I think it's good that we're having this discussion in
> general.  Thanks to everyone for the civil dialog, as it just makes our
> community stronger.
>
> So it seems like most people don't want to have multiple heads in the dev
> branch?  I'm still not entirely convinced that this is bad as long as the
> merges to the non-main dev head are taking place during the PR triage when
> one of the members can assure that the tip (or experimental head) is
> pointing to the right changeset, but people seem really opposed to this.
>
> As for big PRs getting merged directly into the main dev branch, I guess I
> still have some reservations.  Basically what we're saying is that once a
> feature goes into the main dev head, it *will* be included in the next
> major release, and we have to block on that release until its reached the
> agreed upon level of "doneness".  But the problem with this is, different
> features merged in will be "done" at different times, so maybe feature A is
> done and its author really wants to get it out to stable, but feature B is
> not yet done and its author needs to go on vacation or doesn't have time to
> write docs for it or something.  What do we do then?  Also, we've had
> situations in the past with pressure to get releases out that were for
> reasons other than the development cycle (e.g. jobs, personal, etc.), which
> is totally understandable, but this leaves us with no other choice than to
> release all of the features instead of picking which one we release.  Big
> pushes like this are usually when strife occurs in our community.  Having
> multiple heads avoids this clash.
>
> I agree with Matt that API stability is a main concern about merging
> something into dev.  But once something goes into dev, there are going to
> be a lot of "users" of it, and to have the API still open to modifications
> is going to mess with a lot of people's scripts.  So I think in order for a
> PR to get merged into the main dev head, the *doneness* of the
> functionality should be such that the API is stable (with Matt's caveat
> that if it really has to change, then it really has to).  This API stable
> rule will help prevent a lot of problems with users getting frustrated down
> the road.  But perhaps it takes a lot of collaborative work (ie forks of
> forks) before one can get to a stable API before merging into the single
> dev head model.  With multiple heads, this is more easily avoided.
>
> Lastly, I think at minimum, there needs to be some minimum docs
> description (docstrings and a simple recipe) included when major work goes
> into the single-head dev branch, for the simple reason that the other
> developers need to know how to use the functionality to be able to test it
> and contribute to it.  I think it is unrealistic for the other developers
> to have to read through the entirety of the new code to understand how to
> use the functionality once it is in dev.  I know this opinion is
> controversial in our group, but I really think it is important for
> fostering collaborative development with each other.
>

This opinion is absolutely not controversial.  You are not alone in feeling
this way, and I personally cannot think of anyone who disagrees with you
that we need a "minimum docs description" specifically docstrings and a
simple recipe (often included in the docstrings).  I have spent the last
little while looking over pull requests, and nearly all came with this
level of documentation, if not more, at their initial outset post-WIP.

-Matt


>
> Cameron
>
> On Thu, Sep 10, 2015 at 3:29 PM, Matthew Turk <matthewturk at gmail.com>
> wrote:
>
>> Hi,
>>
>> I think identifying the criteria for "doneness" is a way to treat the
>> problem, not the symptom.  I think much of the problem here is in deciding
>> when a feature can go in to a branch or line of development that is
>> pre-release, but somewhere above "totally unfinished."
>>
>> The biggest issue may be related to API stability.  I do not think that
>> we can make a hard and fast rule, but if we were able to identify that "We
>> expect no further API breakages, but will do so if necessary as decided
>> through peer review" would that be enough for major features to land in the
>> "yt" branch?  I do not think we can expect full documentation, but having
>> this compromise may help increase visibility, improve the overall testing
>> of new features (by putting them in a main line of development) and also
>> ensure that they aren't *so* broken.  By this standard, we would not merge
>> VR refactor in yet, but it would be clearer how to do so.
>>
>> -Matt
>>
>> On Thu, Sep 10, 2015 at 5:18 PM, Nathan Goldbaum <nathan12343 at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Thu, Sep 10, 2015 at 4:17 PM, Britton Smith <brittonsmith at gmail.com>
>>> wrote:
>>>
>>>> Hi Andrew,
>>>>
>>>> Have we continued to have problems with PRs being issued to the wrong
>>>> head now that Bitbucket tags the heads with bookmarks?  If so, then maybe
>>>> that's the end, but I was under the impression that that had taken care of
>>>> this.
>>>>
>>>
>>> I don't think it has come up since that hapenned, but we haven't been in
>>> a situation where the "experimental" bookmark was the tip of the yt branch
>>> for more than a day or two.
>>>
>>>
>>>>
>>>> As far as allowing development to take place in a single dev head, I am
>>>> worried about ending up in the situation where many new features in there
>>>> are ready to go and some are not.  If development has continued over
>>>> multiple PRs on different features, will it still be possible to pick out
>>>> only the features that are ready?  If so, then I guess that's that, but if
>>>> not, then I am very wary of this.
>>>>
>>>
>>> In practice how would this problem work? Right now we have an
>>> experimental bookmark including both unstructured mesh support and the
>>> volume rendering refactor. Since unstructured mesh support depends on the
>>> VR refactor, we can't release that it without also releasing the VR
>>> refactor. In principle we could release the VR refactor without
>>> unstructured mesh support, but I think we all agree that 3.3 should
>>> probably include both features.
>>>
>>> I think merging a new feature into a single-headed yt branch implicitly
>>> means that that the feature will be "done" in time for the next major
>>> release. If we have more than one work-in-progress features due for the
>>> next major release, then the release is blocked on all of them. As a
>>> community we need to be careful to not merge in features if we're not
>>> prepared to block the release on them until they are done.
>>>
>>> Maybe as a way to move this discussion forward, since there seems to be
>>> two main perspectives, is there any way at all for us to compromise? I'm
>>> not sure there is, since some of us think it's ok to have work-in-progress
>>> features in the dev branch while they're still not fully baked, while
>>> others strongly object. Maybe we could have a discussion about what level
>>> of "doneness" a new feature needs to have before it can be merged in? Right
>>> now the level of acceptable "doneness" is quite high, and maybe it can be
>>> relaxed a bit while still keeping everyone happy.
>>>
>>>
>>>> Britton
>>>>
>>>> On Thu, Sep 10, 2015 at 4:59 PM, Andrew Myers <atmyers2 at gmail.com>
>>>> wrote:
>>>>
>>>>> I just want to chime in to say I agree with Nathan and Matt re:
>>>>> allowing more active development on the yt branch. Ideally, I think there
>>>>> should be two branches (stable and yt) with one head each, with the yt
>>>>> branch being the "development" version of the code. Most users would be on
>>>>> the stable branch, and more active users / developers would be on the
>>>>> development branch. Having multiple heads / bookmarks on the yt branch
>>>>> makes for a more complicated development process (as evidenced by the fact
>>>>> that none of us can remember how it works in this very email chain ;) ),
>>>>> which can be discouraging for new developers. Since creating the
>>>>> experimental bookmark for the scene refactor, we've already had PRs
>>>>> accidently made and merged into the wrong head, and it's created more work
>>>>> for Kacper in keeping the CI working smoothly.
>>>>>
>>>>> I appreciate the concerns about release deadlines creating pressure to
>>>>> release under-documented and under-tested code. However, and as others have
>>>>> stated, with Nathan's backport script, we can now do bugfix releases as
>>>>> often as needed, which removes some (most? all?) of that pressure.
>>>>>
>>>>> Additionally, I agree that in practice, most of the serious testing of
>>>>> new features is going to be done after the code gets merged, and the
>>>>> community of yt developers / active users who use the development version
>>>>> of the code get their hands on it. Doing that merge sooner rather than
>>>>> later will result in an overall faster timeline to stable code.
>>>>>
>>>>> To be clear, I think Britton's proposal is a definite improvement over
>>>>> the "forks of forks" model, but I think an even simpler development process
>>>>> with only one head on the main yt branch would be more productive in the
>>>>> long run.
>>>>>
>>>>> On Thu, Sep 10, 2015 at 1:23 PM, Matthew Turk <matthewturk at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Hi Britton,
>>>>>>
>>>>>> I was going to reply, broadly in support of everything you suggested,
>>>>>> until I saw the other emails.  It looks like I missed my opportunity.
>>>>>>
>>>>>> In general, I would like to see more experimental development in the
>>>>>> main yt repo; I think with the backporting script, as you suggest, we now
>>>>>> have a good reason for people to use "stable" instead of the main
>>>>>> development branch.
>>>>>>
>>>>>> So I guess "multiple heads" even with multiple bookmarks is off the
>>>>>> table now, if I read the rest of the thread correctly?  If so, can we
>>>>>> figure out a way to allow experimental stuff into "yt" and then move most
>>>>>> folks onto "stable"?
>>>>>>
>>>>>> On Thu, Sep 10, 2015 at 1:52 PM, Britton Smith <
>>>>>> brittonsmith at gmail.com> wrote:
>>>>>>
>>>>>>> Hi everyone,
>>>>>>>
>>>>>>> I had some ideas for improving the yt development process that I
>>>>>>> wanted to run by everyone.  This can be discussed further at our
>>>>>>> upcoming team meeting and if people are in favor, I will issue a pull
>>>>>>> request to the relevant YTEP.
>>>>>>>
>>>>>>> STATEMENT OF PROBLEM
>>>>>>> Currently, development proceeds roughly as follows.  The two main
>>>>>>> active branches within the central yt repository are *yt* and
>>>>>>> *stable*.
>>>>>>> The tip of *stable* is the latest release and the *yt* branch is
>>>>>>> the de
>>>>>>> facto "development version" of the code.  Until recently, we have not
>>>>>>> been very good at regularly scheduled minor releases and so the
>>>>>>> *stable*
>>>>>>> branch sits for quite some time with many bugs that are fixed within
>>>>>>> the development branch.  This effectively makes *stable* unusable
>>>>>>> and
>>>>>>> pushes most users to the *yt* branch.
>>>>>>>
>>>>>>> When new features are developed, pull requests are issued to the
>>>>>>> single head of the *yt* branch.  Because this is the version most
>>>>>>> people
>>>>>>> are actually using, the current policy is to not allow PR with new
>>>>>>> functionality to be accepted until they are 100% ready (full
>>>>>>> functionality, tests, docs, etc).  As we have already seen, this
>>>>>>> makes
>>>>>>> collaborative development very cumbersome, as it requires people to
>>>>>>> create forks of the fork from which the PR originates.  They then
>>>>>>> must
>>>>>>> issue PRs to that fork after which time the original PR is updated.
>>>>>>> The current volume render refactor is the perfect example of this.
>>>>>>>
>>>>>>> PROPOSED SOLUTION
>>>>>>> Before I lay out the proposed solution, I want to list a number of
>>>>>>> recent developments that I think will make this possible:
>>>>>>> 1. Nathan's new script for backporting changes now keeps *stable*
>>>>>>> and *yt*
>>>>>>>    synced on bugfixes.
>>>>>>> 2. We have returned to doing minor releases containing only bugfixes,
>>>>>>>    thanks again to Nathan's hard work.  This and point 1 means that
>>>>>>>    users are once again safe to be on *stable*, and now *should* be
>>>>>>> there
>>>>>>>    most of the time.
>>>>>>> 3. Bitbucket now supports bookmarks, meaning that PRs can be issued
>>>>>>> to
>>>>>>>    specific bookmarks instead of to branches or heads named only by
>>>>>>> the
>>>>>>>    changeset hash.
>>>>>>> 4. The weekly PR triage hangouts are making it easier to process PRs
>>>>>>>    and also providing a place to strategize getting larger PRs
>>>>>>>    accepted.  Thanks to Hilary for keeping this going.
>>>>>>>
>>>>>>> With the above in mind, I propose the following:
>>>>>>> 1. Create a "development" bookmark to sit at the tip of the *yt*
>>>>>>>    branch.  All PRs containing relatively small new features are
>>>>>>>    issued to this.  The requirements for acceptance remain the same:
>>>>>>>    PRs accepted to "development" must contain all intended
>>>>>>>    functionality and be fully documented.  This allows the
>>>>>>>    "development" bookmark to be defined explicitly as everything that
>>>>>>>    will be included in the next major release.
>>>>>>> 2. Large new features should have a corresponding YTEP that has been
>>>>>>>    accepted.  After the YTEP has been accepted, a PR should be issued
>>>>>>>    to the *yt* branch.  After some initial discussion, this PR is
>>>>>>> pulled
>>>>>>>    into the main yt repo with a bookmark named after the feature.
>>>>>>>    Once this has happened, developers can now issue new PRs
>>>>>>>    specifically to this bookmark.  This is effectively what we have
>>>>>>>    now with the volume render work in the "experimental" bookmark,
>>>>>>>    only we would rename the bookmark to something like "vr-refactor".
>>>>>>>    As with PRs issued directly to "development", only after the new
>>>>>>>    feature is 100% ready shall it be merged into the "development"
>>>>>>>    bookmark.
>>>>>>> 3. We continue to make use of the PR triage hangouts to establish
>>>>>>> when
>>>>>>>    large features are ready to be merged.
>>>>>>>
>>>>>>> I believe this will have the following benefits:
>>>>>>> 1. Large, new features can be developed collaboratively without the
>>>>>>>    need for forks of forks of forks.
>>>>>>> 2. New, underdevelopment features are more accessible to the larger
>>>>>>>    community by simply updating to named bookmarks from the main repo
>>>>>>>    (no need for "just pull these changes from my fork").
>>>>>>> 3. The "development" branch is preserved as a place only for
>>>>>>>    ready-to-be-released features (i.e., polished and documented).
>>>>>>>
>>>>>>>
>>>>>>> All told, this is really just a small tweak on our current process.
>>>>>>> Please comment with any thoughts, or even a +/-1 if your feelings can
>>>>>>> be summed up thusly.
>>>>>>>
>>>>>>> Britton
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> yt-dev mailing list
>>>>>>> yt-dev at lists.spacepope.org
>>>>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> yt-dev mailing list
>>>>>> yt-dev at lists.spacepope.org
>>>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> yt-dev mailing list
>>>>> yt-dev at lists.spacepope.org
>>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> yt-dev mailing list
>>>> yt-dev at lists.spacepope.org
>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>>>
>>>>
>>>
>>> _______________________________________________
>>> yt-dev mailing list
>>> yt-dev at lists.spacepope.org
>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>>
>>>
>>
>> _______________________________________________
>> yt-dev mailing list
>> yt-dev at lists.spacepope.org
>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>
>>
>
>
> --
> Cameron Hummels
> NSF Postdoctoral Fellow
> Department of Astronomy
> California Institute of Technology
> http://chummels.org
>
> _______________________________________________
> yt-dev mailing list
> yt-dev at lists.spacepope.org
> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.spacepope.org/pipermail/yt-dev-spacepope.org/attachments/20150911/d8ff6dc0/attachment.html>