Proposal: Approval process for pull requests

[jdmgoogle]

As one of the biggest offenders of the proposal outlined below, I'll attempt to have my "Nixon going to China" moment. :)

The proposal:

Before any pull request can be merged into either the main branch or a release branch (e.g., "vip5") it needs to be affirmatively approved by at least one person from Pew and one person from Google. An "affirmative approval" can be as simple as a "LGTM" (looks good to me).

The rationale:

I'm not trying to push anyone out or say that I don't value/trust/rely on the judgement of people from outside of Pew or Google; in fact it's just the opposite, in that this project simply doesn't succeed without the input and participation of people from Democracy Works, CTCL, state and local election officials, and everyone else. As a Pew/Google project, though, this is the bed we need to lay in (and maintain) so I'd just like at least one person from each organization to have glanced at incoming PRs and be able to say "yes, we can work with that."

The discussion:
@jungshadow @pstenbjorn @nomadaisy @pkoms @djbridges @cjerdonek @jktomer @demcg @jen-tolentino @MariaDemocracyWorks @kennethmbennett @Josh-LACRRCC

[cjerdonek]

So people can know who approvals would be needed from, what are the usernames of the people from Google? How about Pew?

[jdmgoogle]

Google: @jdmgoogle @jktomer
Pew: @jungshadow @jen-tolentino
Wildcard: @pstenbjorn

The Pew people should probably say if they want to include @pstenbjorn on the Pew list because I'm not 100% certain about his relationship to Pew. I'm fine either way, though.

[cjerdonek]

I just want to make one comment that, with this policy, it might lessen incentive for non-Google, non-Pew people to say LGTM since their positive feedback wouldn't contribute towards the needed approval.

[demcg]

Sounds good, but in the case of issues with healthy debate can we allow the debaters time to weigh in on the PR as "observers"?

[jdmgoogle]

It's not my intention to lessen the value of other people's opinions and contributions towards issues and pull requests. Non-Google, non-Pew people have both real-world experience which Googlers and Pew..ers(?) may not have, in addition to technical expertise. I certainly rely on the technical knowledge and real-world experience of the people here, and their feedback and comments most certainly contribute to the final decision.

The goal of this policy is more of a slight refinement of what @jungshadow put forth, in that we should have multiple sets of eyes on any issue or PR, and at least one set of eyes should come from each of the organizations which are ultimately responsible for the project.

[pstenbjorn]

@jdmgoogle I am all for the approval process for PRs and I would add that an additional step that someone must assume before merging a PR is to validate the schema using a W3C validation routine (there are some online, there are some IDEs that perform this and there is a plugin for Notepad++ that can validate any schema against a parent schema).

[jdmgoogle]

@pstenbjorn Agreed. I'll update the contributors document to provide links to http://www.utilities-online.info/xsdvalidation/ and xmllint.

[jdmgoogle]

See PR #123 for updates to the documentation. This is the formal version of this proposal, bug please keep comments here about the idea itself.

Also, I did a validation of the most recent vip5 schema and found that it didn't validate. See PR #122 to fix that.

[jktomer]

This requirement LGTM. I'll echo that we definitely want to be hearing from folks outside Pew and Google! But as the primary producer and (to my knowledge) largest (by several orders of magnitude if not infinity) direct consumer of VIP feeds, respectively, Pew and Google bear long-term maintenance responsibility for most of the actual software that will be interacting directly with data in this format, so for practical purposes it makes sense for those two organizations to be "owners for review purposes."

[jdmgoogle]

The new PR process is documented and has been fed into vip5.