AcademySoftwareFoundation
OCIO development process documentation feedback
A new document PROCESS.md was recently added to the repository documenting project practices around proposing changes to the core library and how small and large changes are approved before being merged into the codebase.
This document was raised for discussion at three OCIO TSC meetings and was available for reivew in the original pull request ( #1140 ), but in an effort to have wider community engagement I'm opening this issue to invite additional feedback from anyone with interest or opinion on what was documented. This is important because it defines how the project operates and what checks and balances are in place to guard to code. This document was made in response to issue ( #1124 ) for those not following the original discussion.
Please chime in if you have comments, and any feedback will be discussed in this issue and in a future TSC meeting (which all are welcome to attend) to determine if any further edits are necessary. If there is no additional feedback after two weeks we will close this issue and keep the document in its current state.
Thanks all.
To go back to https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1158#issuecomment-701745081 and my OP here https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1140#issuecomment-701159344
The sections @KelSolaar flagged are present in CONTRIBUTING.md even with this reverted, so the policy is present in either case.
Should this be made explicit in the PROCESS.md document?
So to reiterate your point from #1140 @KelSolaar , you are suggesting that we should not allow two affiliated approvals ((i.e. from two developers from the same company) in the two week PR review clause, correct? And to restate your argument, which is a fair point:
Isn't this still empowering a group of affiliated people to steer the project where they want? It is Christmas break, Michael is taking some holidays, and we get a raytracer in OCIO at New Year's Eve, because, well nobody really looked at the project... :) I think that Core changes should not have an escape door.
I don't really have a dog in this fight, but it seems to me that no matter what hooliganism occurs over Christmas break, on January 2nd anybody in the community can say, "whoa, that commit from last week was a hot mess, let's revert it and take some time to rethink this feature." It would only be a real problem if it happened immediately prior to a release, allowing the bad ideas to escape into the wild and have to be supported. But you can fix that by deciding that there should be a design/feature freeze for a few weeks before major release and give extra scrutiny during that period to any last-minute changes and also to allow any iffy ideas from the past to get edited while there's still time.
All of this sounds quite reasonable to me. Informative and well written.
Isn't this still empowering a group of affiliated people to steer the project where they want?
I definitely haven't given this nearly as much thought as others have, and I may be failing to imagine problematic scenarios... but in my mind, provided the group isn't disproportionately represented in the TSC, and contributors are communicative and responsive to feedback / questions / suggestions communicated in GitHub... if the alternative is PR approval limbo and crickets, I'm not sure I have a problem with empowering a group of affiliated people to drive the project. I feel like there are sufficient precautions in place to keep OCIO from being hijacked anywhere it wasn't roughly headed toward already. And like Larry says, we can always make u-turns (although it's helpful not to land first). This has not been my finest metaphor.
On the same token, if there's controversy surrounding a proposed feature, ideally, those concerned or affected would summarize / argue cases for or against publicly on GitHub. If all it takes is an active objection to keep a PR from being automatically merged after two weeks, then I'd imagine the PR would be held off until a consensus is reached. Maybe I'm wrong.
I don't want to disincentivize or frustrate well-communicated, non-breaking efforts towards TSC-kosher contributions (from anyone); and I do want to encourage and reward constructive feedback and open discussion. I think the process guidelines reinforces productive, prosocial behavior while limiting (although not eliminating) the risk of... (puts on eyepatch)... corporate mutiny.
Also, this wasn't clear to me -- do I have the ability to give grey-star "approvals"? I like that idea. There are times I've inspected PRs to grok what's going on; and although it wouldn't be appropriate for me to give bona-fide code reviews for a language I barely have experience with, I would like to communicate my approval of general ideas / mechanisms / apparent implementations without having to type stuff.
I don't really have a dog in this fight, but it seems to me that no matter what hooliganism occurs over Christmas break, on January 2nd anybody in the community can say, "whoa, that commit from last week was a hot mess, let's revert it and take some time to rethink this feature."
I don't disagree but at this stage, the community is tiny and depending on its availability, it might take a while to realise that stuff happened. It relates to what Mark (@Shootfast) was saying at the last TSC, depending on your workload, you might simply not have time to check OCIO development as much as you would want but still, it is a critical piece in your work. I actually write that on a Sunday at 10pm because it is the only time I found this week to look back at what happened.
But you can fix that by deciding that there should be a design/feature freeze for a few weeks before major release and give extra scrutiny during that period to any last-minute changes and also to allow any iffy ideas from the past to get edited while there's still time.
That would work well for OIIO where you have a quite regular release cadence but OCIO is at orders of magnitude slower pace.
OCIO v2 is years in the making. It seems like it would be easy, barely a blip in the schedule, to say that the last 6 weeks before declaring it final is a design and feature freeze -- no new ideas or unvetted features -- with the only changes to be bug fixes, completion of promised features that are documented/speced out but not fully implemented, and corrective responses to any community concerns about the design.
Agreed but at some point people might have started to build around the new features in their software and it could be challenging to ask for any change, case in point some discussions around some of CLF operators, e.g. Range, faced a lot of resistance to make any changes.
@zachlewis you are definitely encouraged to give approvals for any review you do and approve of, yes. All feedback and opinions are valued.
Thanks for the input all, including in last week's TSC meeting. Sounds like we can add some policy around feature freezes, and also consider future revisions to the approval policy after OCIO v2 releases, under the intent that with a reduction in the cadence of major changes to the library there is likely more time and cycles from the community and committers for in-depth review, so hopefully less need for an escape hatch. I'll work on some revisions in the coming weeks and submit another PR, referencing it here for further open discussion.