Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into rfc-1-comment-1
Browse files Browse the repository at this point in the history
  • Loading branch information
joshmoore committed Apr 24, 2024
2 parents 96a9498 + c7a6360 commit 604976c
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 1 deletion.
2 changes: 1 addition & 1 deletion .codespellrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
skip = .git,.codespellrc
check-hidden = true
# ignore-regex =
# ignore-words-list =
ignore-words-list = commend,Fuchsia
91 changes: 91 additions & 0 deletions rfc/1/review_2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# RFC-1 Review

## Contributors

The contributors to this review were Davis Bennett, John Bogovic, Michael Innerberger, Mark Kittisopikul, Virginia Scarlett, and Yurii Zubov (referred to in this document as ‘the reviewers’). This review does not represent the opinions of any other individuals, nor of HHMI Janelia as an institution.

Please note that the reviewers had mixed opinions on whether a collective review was appropriate. The reviewers proceeded as a courtesy to the author, but clarification on this point is urgently needed.

## Decision: Major Changes (at best)

Davis Bennett: Reject
John Bogovic: Major changes
Michael Innerberger: Major changes
Mark Kittisopikul: Reject
Virginia Scarlett: Reject
Yurii Zubov: Major changes

## Summary of RFC-1

In this proposal, the author describes a regulatory process for making revisions to the OME-NGFF specification, adapted from the RFC process used by the Internet Engineering Task Force (IETF). The IETF RFC process has been used to produce technical documents describing the core specifications and protocols of the Internet, including ASCII ([RFC 20](https://www.rfc-editor.org/info/rfc20)), Hypertext Transfer Protocol (HTTP 1.0, [RFC 1945](https://www.rfc-editor.org/rfc/rfc1945.html)), and Hypertext Markup Language (HTML 2.0, [RFC 1866](https://www.rfc-editor.org/rfc/rfc1866.html)).

Note that the proposal being reviewed is itself an RFC.

Broadly speaking, the proposed process consists of three phases:

1. A DRAFT phase, in which an RFC (a proposal) is created as a PR to a subfolder of the OME-NGFF repository. The PR is discussed, and stakeholders offer critiques and/or endorsements. Finally, after some comments and edits, the RFC PR is merged.
2. A REVIEW phase, in which the RFC is evaluated by several expert Reviewers. Reviewers are chosen by Editors.
3. A SPEC phase, in which the proposal is accepted as part of the specification.

Minor revisions would not require an RFC. Timelines are suggested or required for each of the above phases.

The impetus for establishing such a process came from the OME-NGFF community, wherein many individuals felt that slow, ambiguous review processes were impeding acceptance of new changes and discouraging potential contributors.

## Review Abstract

The reviewers commend the author for proposing a well-researched solution to an important and complex set of challenges that have arisen within the OME-NGFF project. The reviewers are particularly pleased with the inclusion of deadlines, which will help changes move forward in a timely manner. However, the reviewers feel that the proposal needs major changes. It is not the reviewers’ intention to antagonize the author, but rather to give OME-NGFF stakeholders another chance to go back to the drawing board together. Including contributors and implementers in the early brainstorming process will help OME-NGFF achieve a decision-making framework that adds the right amount of structure and has broad community support.

## Full Review

The reviewers are grateful to the author for taking the initiative to craft this proposal, and for soliciting reviewer feedback. The proposed process is detailed but straightforward. It emphasizes the values of accountability and transparency, which are excellent goals. The reviewers agree that the greatest strength of this proposal is that it gives contributors clear expectations through a defined process and offers *formal deadlines* for each stage. It is expected that these deadlines, tied to each of the three RFC phases, will provide Authors with more structured and stable feedback than might be expected from comments on GitHub.

Nevertheless, the reviewers felt that the proposal should not move forward unless major changes are made. The reviewers especially wish to emphasize the desire for greater clarity on the existing problems that this RFC is meant to solve, and why the proposed process is the best way to solve them.

### Scope of the Proposed Process

Some reviewers were concerned that the process described by this RFC does not adequately address the OME-NGFF community's most pressing concerns. As such, this section is more about the scope of this RFC than its content.

Currently, minor changes to the OME-NGFF specification are not being accepted, while major changes are moving slowly, leading to low morale and risk of contributor burn-out. Consider two cases of stalled revisions:

- The [discussion about units](https://github.com/ome/ngff/pull/168)
- The [table specification proposal](https://github.com/ome/ngff/pull/64)

While a detailed analysis of these case studies is beyond the scope of this review, suffice it to say that the proposed RFC process would not have helped either of them much. The discussion of units would have been too minor to warrant an RFC. For the table specification, the RFC process would have drastically reduced the time spent on the proposal, which is good. However, what effectively ended the proposal was that one community liked the table specification and another community didn’t. While the proposed RFC process includes mechanisms for rejection of one solution and acceptance of another, it is not clear that this is the best solution when conflict arises between domain experts using similar data structures for distinct applications.

Upon reflection, it becomes clear that proposed revisions to the specification can be major or minor, as well as generic or domain-specific, and these two dimensions have important implications for decision-making. The reviewers would like greater clarity on how this process would be applied to different proposals that vary along these two dimensions. Clarity along the major/minor dimension is discussed further in ‘Major Issues’, below.

The reviewers recommend that the author and OME-NGFF stakeholders conduct a retrospective exercise to identify the high-priority problems, and then propose the *minimal amount of structure needed to tackle those particular problems*. A record of this retrospective exercise might belong under RFC-0.

The reviewers wish to emphasize that while this review contains many criticisms, it is not intended as a reprimand. The reviewers simply feel that a community discussion, presumably conducted through one or more **virtual meetings**, is vital to developing a procedural framework that will satisfy as many OME-NGFF stakeholders as possible. Since this RFC is already at the REVIEW stage, the only recourse the reviewers have to reinvigorate community discussion is to recommend major changes or rejection.

Below, the reviewers outline major and minor critiques. Some of the critiques suggest tweaks to the existing proposal, but the reader should not take these to mean that the reviewers recommend tweaking the proposal. Scrapping the entire proposal may be more appropriate, in which case many of the comments below will be moot.

### Major Issues

- First, it is not clear to the reviewers that the RFC model is appropriate. The RFC process may introduce more bureaucracy than is needed for this relatively small community (compared to the IETF), which is furthermore still in version 0.x, an early phase [generally associated with rapid iteration](https://semver.org/#spec-item-4). The reviewers recommend holding virtual meetings to discuss alternative procedural frameworks. In particular, the reviewers find three (non-mutually exclusive) alternatives compelling*. These include: (1) developing a group charter as is common for W3C and IETF working groups; (2) simply curating PRs according to a well-defined scope and editor discretion; and (3) distinguishing between core functionalities and domain-specific extensions. These alternatives employ distinct operational procedures, but they are similar in calling for a clear, simple, and explicit direction of OME-NGFF that would allow the group to achieve core goals while permitting future extension.
- *Note that there were six reviewers, and there was disagreement among the reviewers about which alternatives would be best. Pursuing a single alternative may not satisfy all reviewers.
- The proposal does not specify how Reviewers and Editors are to be selected. If the Reviewer selection process is arbitrary, then the RFC system does not seem to offer much advantage over the repository owner simply choosing maintainers at their own discretion, as is common for many open-source projects. If the selection process is not arbitrary, then clarity is needed on the criteria for choosing an appropriate Reviewer or Editor, such as the level of interest, investment, and expertise they ought to have in the proposed change, and whether one individual may hold multiple roles.
- The proposal is vague on the criteria for determining whether a change is big enough to warrant an RFC. As discussed in the Scope section above, distinct procedures might be appropriate for major and minor changes.*
- *Again, there was some disagreement among reviewers. One reviewer countered that at [stage 0.x](https://semver.org/#spec-item-4), no changes are truly major, and therefore no change is big enough to warrant an RFC.
- The early iteration and vetting stages, specifically (D4), might benefit from more structure. This structure should clarify authors’ responsibilities in relation to comments. For example:
- Specifying a very clear cut-off, either temporal and/or in terms of scope, between comments the author must address and comments they can safely ignore.
- Specifying how many endorsements are needed, as well as how to record a non-endorsement (objection). Clarifying the implications of a certain ratio of endorsements to objections, or objections of a certain kind.
- Offering a mechanism for calling a meeting if the GitHub discussion becomes unwieldy.
- Explanation is needed as to why ‘Reject’ decisions "should be a last recourse," and what steps will be taken to ensure that a PR would likely be accepted if it becomes an RFC.
- How are deadlines enforced? What is the protocol for extending deadlines in case of emergency, and/or non-emergency? Clearer policies are needed here.
- The topics of versioning and extensions are avoided. If these issues are not resolved now, then when?

### Minor Issues

- The reviewers are pleased that implementers are given special attention as Reviewers. However, care should be taken to not let pre-existing implementations dictate the strategic direction of NGFF.
- Can the ‘SPEC’ phase simply be eliminated? This is not so much a phase as a one-time event.
- What is the purpose of the metadata model in (G)? Is this something that might be implemented later? If so, its inclusion at this stage seems premature.
- Publishing reviews on preprint servers may not be a good idea. For one thing, this would substantially scatter communication about RFCs.
- The Author might consider making the ‘Implementation’ and ‘Tutorials and Examples’ sections required.

### Writing Style Considerations

- The template has many sections that seem irrelevant, e.g. ‘Security Considerations’ and ‘Typeface’.
- In the diagram, only ‘SPEC adopted’ is a proper leaf of the graph. ‘RFC persists’ and ‘PR closed’ seem to feed back to D2 and D3, respectively, so that an infinite loop is created.
- It would help if there were an extremely basic overview of the process at the top of the document—either a paragraph or a graphical abstract. The current diagram is useful, but is too detailed to quickly convey the important parts of the process.
- Each of the ‘phases’ sections starts with *a description of what happens* in that phase, but the reviewers recommend starting with the *purpose* of that phase.
31 changes: 31 additions & 0 deletions rfc/1/review_3.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# RFC 1

This review submitted by Matthew Hartley, on behalf on EMBL-EBI's imaging data resources (BioImage Archive, EMPIAR and EMDB).

## Summary

As OME-NGFF grows together with its community, mechanisms for formalising the process by which the specification is updated have become necessary. The proposed process follows models which have worked effectively for large open source projects with invested communities. It establishes clear timelines and makes expectations for involved parties clear.

Enabling a clear record of the decision making process for both successful and failed RFCs is a particularly important feature, given that thus-far discussion is currently scattered across PRs, forum posts and hackathon notes. This makes onboarding new members of the community difficult, if they are to be expected to understand prior specification decisions and processes. Additionally, the mechanisms by which the process allows its own updating through future RFCs should help to keep it fit for purpose in the future.

Developing sharing specifications, like any large scale shared endeavour, necessarily involves compromise across the needs of different groups. It will not be possible to find a communal specification development process that completely satisfies all stakeholders. With some minor changes and, most critically, sufficient will from the community to participate in good faith, we believe that that the proposed process has excellent potential to allow the specification to move forward.

## Significant comments and questions

* In the DRAFT phase, it is somewhat unclear how D4 (“Questions raised during PR review?”) is evaluated - comments on PRs can vary in how clear their intent is to be actionable feedback. If the intent is that the editor(s) have the final call here, it would be useful to indicate this.
* The proposal makes reference to manuscript review (particularly for the REVIEW phase). In this process, time overruns or nonresponsive participants are common. Some further detail on how such cases would be handled and communicated would be valuable.
* Both within the REVIEW and SPEC phases, implementations become important (particularly for reaching “adopted” status). However OME-NGFF is a complex specification and in many cases existing implementations already focus on subsets of the spec. We recommend adding some general indication of what would comprise a minimal implementation necessary to enable adoption.
* Given exit from the SPEC phase is dependent on complete implementations, care should be taken on how to prevent the RFC getting stuck here.
* The RFC template needs a revision pass, it has a number of components which are very specific to the project from which the template was copied (presumably the Fuchsia project), e.g. explicit mentions of “FIDL source file compatibility”. More broadly, removing some template sections (e.g. security/privacy/UI/UX), would make sense.

## Minor comments and questions

* During the RFC process, “Reviewers responses should be returned in less than two weeks.” I think this means the responses by the authors to the reviewers? Phrasing is unclear.
* The role of **Commenters** is a little unclear to me at present. In the definition, they are “invited to add their voice” - is this intended to be an active (e.g. authors/editors ask them) or passive process?
* In the figure “RFC persists” indicates that the RFC, although not adopted, remains part of the record of communal work on the specification, but “persists” suggests that it remains in an active process somehow which I do not think is the case.
* The figure legend suggests that start/end states should have identifiers, this isn’t currently the case for end states.
* Very nitpicky - text in some of the state boxes in the figure terminate in “.” characters, others don’t.

## Recommendation

Given the clear need for a process by which the OME-NGFF specification can evolve, and the careful thought and parallels to tried-and-tested processes evident here, we recommend adoption with **Minor changes**.

0 comments on commit 604976c

Please sign in to comment.