Details

    • Type: Task Task
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: docs
    • Labels:

      Description

      We currently don't have any guidelines on how we should be using reviewboard.

      For example – How many "ship it"'s are needed to merit consensus? How do we make sure that reviews get applied after consensus is reached?

      Figure out what to do and then write it down.

        Issue Links

          Activity

          Hide
          John Vines added a comment -

          I think similar rules to our consensus voting is sufficient - 2 binding (committer) "ship it's" are sufficient. I don't think a defined time period is necessary because it can easily be reverted, so if there is an issue with it, it can be brought up before it's committed, or afterward if there's a delay.

          Show
          John Vines added a comment - I think similar rules to our consensus voting is sufficient - 2 binding (committer) "ship it's" are sufficient. I don't think a defined time period is necessary because it can easily be reverted, so if there is an issue with it, it can be brought up before it's committed, or afterward if there's a delay.
          Hide
          Bill Havanki added a comment -

          Whatever the rules, I think they should complement the rules for committing patches submitted to JIRA. The more rigorous the rules for RB end up, the less they should be for patches submitted after review, so that committers don't have to exert the same effort twice. If a patch doesn't go through RB, it should face more scrutiny before commit.

          The Apache voting rules say 3 +1s for a code mod, but that seems high. I like 1 or 2 ship-its. Maybe if it gets only 1, the patch committer must not be the 1 ship-it.

          What changes don't need to go through RB? Documentation? Commenting?

          Show
          Bill Havanki added a comment - Whatever the rules, I think they should complement the rules for committing patches submitted to JIRA. The more rigorous the rules for RB end up, the less they should be for patches submitted after review, so that committers don't have to exert the same effort twice. If a patch doesn't go through RB, it should face more scrutiny before commit. The Apache voting rules say 3 +1s for a code mod, but that seems high. I like 1 or 2 ship-its. Maybe if it gets only 1, the patch committer must not be the 1 ship-it. What changes don't need to go through RB? Documentation? Commenting?
          Hide
          Billie Rinaldi added a comment -

          Requiring a specific number of +1s seems contrary to our CTR model. Using reviewboard is a voluntary method of requesting more eyes on your code, so I don't think we should make RB patches harder to commit than other patches that have just been attached to tickets.

          Show
          Billie Rinaldi added a comment - Requiring a specific number of +1s seems contrary to our CTR model. Using reviewboard is a voluntary method of requesting more eyes on your code, so I don't think we should make RB patches harder to commit than other patches that have just been attached to tickets.
          Hide
          Christopher Tubbs added a comment -

          CTR says reviews aren't required for a commit, but it doesn't rule it out as an option. In these cases (such as an anticipated concern about the readiness or quality of a feature implementation), it seems like a formal vote is still an option. To exercise that option, it seems to me that the formal voting should take place on the mailing list, just like other votes, not in ReviewBoard. ReviewBoard feedback can, of course, be used to inform a committer's decision to call a formal vote.

          Show
          Christopher Tubbs added a comment - CTR says reviews aren't required for a commit, but it doesn't rule it out as an option. In these cases (such as an anticipated concern about the readiness or quality of a feature implementation), it seems like a formal vote is still an option. To exercise that option, it seems to me that the formal voting should take place on the mailing list, just like other votes, not in ReviewBoard. ReviewBoard feedback can, of course, be used to inform a committer's decision to call a formal vote.
          Hide
          Mike Drob added a comment -

          To exercise that option, it seems to me that the formal voting should take place on the mailing list, just like other votes, not in ReviewBoard.

          Comments on RB do go to the mailing list, however, making them just as good in my understanding.

          Show
          Mike Drob added a comment - To exercise that option, it seems to me that the formal voting should take place on the mailing list, just like other votes, not in ReviewBoard. Comments on RB do go to the mailing list, however, making them just as good in my understanding.
          Hide
          Christopher Tubbs added a comment -

          Comments that go to the mailing list don't include [VOTE] in the subject, and don't provide enough context to know whether it is actually something we are formally voting on (via ReviewBoard) or just any old review. At the very least, formal votes that are "[VOTE]" threads should start on the mailing list, I think, even if the actual voting is tallied via the reviews.

          Show
          Christopher Tubbs added a comment - Comments that go to the mailing list don't include [VOTE] in the subject, and don't provide enough context to know whether it is actually something we are formally voting on (via ReviewBoard) or just any old review. At the very least, formal votes that are "[VOTE]" threads should start on the mailing list, I think, even if the actual voting is tallied via the reviews.
          Hide
          Sean Busbey added a comment -

          Moving this off of the 1.6.0 release

          Show
          Sean Busbey added a comment - Moving this off of the 1.6.0 release
          Hide
          Bill Havanki added a comment -

          In light of the bylaw effort, here is some information to help us decide on RB's role. This information is current as of this writing, but the bylaws have not yet been approved.

          Code changes will be approved by lazy approval, meaning automatically after one day except if there is a veto. In case of a veto, consensus approval is required (+3 binding votes, no -1).

          There is no requirement for RtC. No specifics on RtC vs. CtR are in the bylaws, but decisions on that are deferred to a separate document.

          So, I see two roles for RB:

          1. As a way to provide an optional review before committing, as Christopher suggests. This would not absolutely require any ship-its because we start with lazy approval.
          2. As a way to provide a review for consensus approval after a code change veto. As Christopher points out, we may need to post a "[VOTE]" to dev.

          A contributor could use other means for the above, like pointing to patches posted to Jira, but RB is there and may be convenient.

          Show
          Bill Havanki added a comment - In light of the bylaw effort, here is some information to help us decide on RB's role. This information is current as of this writing, but the bylaws have not yet been approved. Code changes will be approved by lazy approval, meaning automatically after one day except if there is a veto. In case of a veto, consensus approval is required (+3 binding votes, no -1). There is no requirement for RtC. No specifics on RtC vs. CtR are in the bylaws, but decisions on that are deferred to a separate document. So, I see two roles for RB: 1. As a way to provide an optional review before committing, as Christopher suggests. This would not absolutely require any ship-its because we start with lazy approval. 2. As a way to provide a review for consensus approval after a code change veto. As Christopher points out, we may need to post a "[VOTE]" to dev. A contributor could use other means for the above, like pointing to patches posted to Jira, but RB is there and may be convenient.
          Hide
          Bill Havanki added a comment -

          I took the liberty of posting a draft set of guidelines, on RB no less. Please feel free to use this as a rough basis for the real deal.

          Show
          Bill Havanki added a comment - I took the liberty of posting a draft set of guidelines, on RB no less. Please feel free to use this as a rough basis for the real deal.

            People

            • Assignee:
              Unassigned
              Reporter:
              Josh Elser
            • Votes:
              2 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:

                Development