OpenYOLO reviewer and maintainer rights

[Iain McGinniss]

Hello all,

Now that we are drawing close to what I believe will be the final, production ready release of OpenYOLO for Android, I'd like to add a little more formality to the review and release process for the code associated with the project.

To date, myself and David Samuelson from Google have been the only two people able to submit code to the OpenYOLO-Android repository. I would like to create a broader "reviewer pool" for submissions, so as to alleviate any concerns about organizational bias - to this end, I'd like to add Michael Verde from 1Password, and Stan Kochen from Dashlane to the pool. 

As part of this, we must also establish the standard that we must all uphold when reviewing and submitting code. I propose the following, as a base set of rules upon which we can build:

- A reviewer from the pool must be assigned for all pull requests within one business day. The reviewer should not be from the same organization, except in circumstances where this is unavoidable due to the availability of reviewers.

- Review comments must be provided within two business days of assignment of a reviewer, or 

follow-up changes provided in response to review comments.

- All code submitted must pass the automated checks defined in our build. Reviewers do not need to more rigorously review code until these checks pass - they can simply reply with "please fix the build errors".

- All code submitted must have at least 60% coverage for the affected lines. Reviewers may request higher coverage, or additional tests generally, if the changes made warrant this.

- Code submitted must not drop the overall coverage of the project below 80%.

- Code changes that have implications for the specification must have an associated thread of discussion here on this mailing list, and consensus must have been reached that the change is worthwhile.

- Code changes that affect the API must also be demonstrated as part of the test and sample applications.

- Code changes that affect the SPI must also be demonstrated as part of the test and demo providers.

- Complex changes may be broken into multiple pull requests for review, but these should not be directly submitted to the master branch - instead, they must be submitted to a feature branch until the complete set of changes is made, and then merged into master.

- In the case of disputes over code style or implementation approach, I will be used as a tie-breaker.

I believe this should be a sufficient set to get us going; if there is agreement on this, I'll add this to the contributor instructions in the repository, and then give Michael and Stan the necessary permissions. to review and submit code.

Iain

[stan....@dashlane.com]

I think this is a great idea and the conditions sounds good for me.

"A reviewer from the pool must be assigned for all pull requests within one business day."

Who assigned a reviewer from the pool to a PR? A reviewer "takes" a PR and assign it to himself?

Stan

[Iain McGinniss]

I was hoping to use a tool to do automatic assignment, but manual self selection should work fine for the time being.

--

---
You received this message because you are subscribed to the Google Groups "OIDF Account Chooser list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to oidf-account-chooser-list+unsub...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[David Samuelson]

Sounds great to me. Once Stan and Michael are granted permission I have a PR that we can begin this new process with. In fact once this is in I believe it would be a good time to cut a new release version before Droidcon London.

To unsubscribe from this group and stop receiving emails from it, send an email to oidf-account-choos...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

---
You received this message because you are subscribed to the Google Groups "OIDF Account Chooser list" group.

To unsubscribe from this group and stop receiving emails from it, send an email to oidf-account-choos...@googlegroups.com.

[David (@dxslly)]

Sorry to piggy back on this thread, but while waiting for the permissions update I have one more very minor PR I'd like to get in before the next release is cut. Stan or Michael, would you mind reviewing? 

On Friday, September 29, 2017 at 9:58:03 AM UTC-7, David (@dxslly) wrote:

Sounds great to me. Once Stan and Michael are granted permission I have a PR that we can begin this new process with. In fact once this is in I believe it would be a good time to cut a new release version before Droidcon London.

To unsubscribe from this group and stop receiving emails from it, send an email to oidf-account-chooser-list+unsub...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

---
You received this message because you are subscribed to the Google Groups "OIDF Account Chooser list" group.

To unsubscribe from this group and stop receiving emails from it, send an email to oidf-account-chooser-list+unsub...@googlegroups.com.

[Iain McGinniss]

As we all seem to be in agreement, I've now added this to the CONTIBUTING.md in the repository:

https://github.com/openid/OpenYOLO-Android/pull/115

Michael and Stan have now been given write access to the repository, so we can add them as reviewers on PRs, and they can merge changes. Please, everyone take a look at the change to the instructions to make sure you understand the process.

Iain

On Monday, October 2, 2017 at 9:55:15 AM UTC-7, David (@dxslly) wrote:

Sorry to piggy back on this thread, but while waiting for the permissions update I have one more very minor PR I'd like to get in before the next release is cut. Stan or Michael, would you mind reviewing? 

On Friday, September 29, 2017 at 9:58:03 AM UTC-7, David (@dxslly) wrote:

Sounds great to me. Once Stan and Michael are granted permission I have a PR that we can begin this new process with. In fact once this is in I believe it would be a good time to cut a new release version before Droidcon London.

[Michael Verde]

I realized after the fact that I communicated my agreement with this plan directly to Iain but forgot to share that back to the mailing list. The updated guidelines in PR#115 look good to me.

Cheers,

Michael