Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure and update community governance docs #2778

Merged
merged 14 commits into from Apr 27, 2020

Conversation

AevaOnline
Copy link
Member

This is a first pass at re-organizing the community documentation. I anticipate a healthy conversation and several iterations of this work before it is acceptable to merge. It's a hefty patch, with substantial implications for the project, and I'd like folks to understand what I am suggesting before we merge this.

Changes:

  • simplifying front page / README
  • creating a docs/Community folder for documentation about community structure/governance
  • refactored the top-level README to be a succinct summary of the project,
    with content mostly moved to docs/GettingStartedDocs/README
  • added docs/Community/README, which outlines the community structure and helpfully links to other documents
  • moved docs/Contributing to docs/Community/Contributing, clarified the
    "Design Discussion" section and added a "Weekly Issue Triage" section
  • refactored docs/Governance into
    docs/Community/Governance (which is also significantly expanded upon)
    docs/Community/ReleaseProcess (mostly unchanged)
    docs/Community/conduct/ConflictPolicy (mostly unchanged)
  • refactored docs/Committers into
    docs/Community/Committers (retains list of committers, though I'd like to remove this)
    docs/Community/governance/charter (retains definition of scope)
    docs/Community/governance/README (retains list of committee members)
  • added a few SIG folders and templates as placeholders and examples
  • added a few other files as placeholders for content that would be helpful to write soon

To get past the license check affecting documentation, I also added a
wildcard match in scripts.check-license.ignore for docs/*

Signed-off-by: Aeva Black 806320+AevaOnline@users.noreply.github.com

@AevaOnline AevaOnline added do not merge Indicates a PR should not be merged until removed. CGC labels Apr 1, 2020
@AevaOnline AevaOnline requested review from achamayou, johnkord and a team as code owners April 1, 2020 09:56
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@dthaler dthaler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good contribution, lots of good things to discuss here.
I think this PR combines many uncontroversial things with some possibly(?) controversial things.
If we don't get immediate consensus, it might be helpful to split this long PR into two so that part of it could be merged immediately.

README.md Outdated Show resolved Hide resolved
docs/Community/Committers.md Outdated Show resolved Hide resolved
docs/Community/Committers.md Outdated Show resolved Hide resolved
docs/Community/Communication.md Outdated Show resolved Hide resolved
docs/Community/Governance.md Outdated Show resolved Hide resolved
docs/Community/ReleaseProcess.md Show resolved Hide resolved
threatening or harassing, report it. We are dedicated to providing an environment
where participants feel welcome and safe.

Reports can be made to any member of the [Community Governance Committee](
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your Community/CodeOfConduct.md lines 8-11 say: "Instances of abusive, harassing, or otherwise unacceptable behavior may be reported to the Open Enclave Community Governance Committee or to the Confidential Computing Consortium."

But here only mentions the former. We should be consistent, whichever we use.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree that this needs to be so formal and consistent. It's community documentation, not a C header file.

==================================

SIG-Attestation is responsible for code, plugins, testing, and other works
related to or required for in-enclave attestation and integration with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove "in-enclave" since there is an experimental "host verification" library for verifying attestation reports outside of an enclave.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, neat!

docs/Community/sig-testing/README.md Show resolved Hide resolved
docs/GettingStartedDocs/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jhand2 jhand2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for writing this up and organizing the community docs! A few small comments

README.md Outdated Show resolved Hide resolved
docs/Community/Committers.md Outdated Show resolved Hide resolved
* on our mailing list or chat server (details TBD) (TODO)

While we strive for openness and transparency, it is natural that some
discussions will also happen in person (eg, in the hallway track) or other
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is just using the conference term "hallway track" as an analogy. Some people that work on the SDK inevitably work in close physical proximity and not all discussions will happen online. Given that reality, I think this is good guidance in terms of decision-making for the SDK.

This is language that is pretty specific to people who attend a lot of conferences (I hear it most often used by academics) so I agree that language could be a little confusing.

@AevaOnline
Copy link
Member Author

Good contribution, lots of good things to discuss here.
I think this PR combines many uncontroversial things with some possibly(?) controversial things.
If we don't get immediate consensus, it might be helpful to split this long PR into two so that part of it could be merged immediately.

Thanks for the feedback, @dthaler . I can certainly see your point that splitting this into multiple PRs might make it easier to merge. I didn't feel that I had the knowledge to guess that ahead of time, so I opened as one PR, and I'm happy to refactor it.

Do you have any suggestions insofar as where to draw those lines?

@achamayou
Copy link
Contributor

@AevaOnline thank you for proposing this new structure and taking the time to update the doc! I do not have any objections, I've left some minor local comments and suggestions inline.

The only general question I have, and perhaps this PR is too early to ask, is how do we reconcile the strict code ownership property of the SIGs (defined per-directory by Prow, as I understand it), and some of the SIGs mentioned in the document or the discussion such as "Release SIG", "Architecture SIG" or "API SIG"?

Would the release SIG only own packaging scripts? Would it own release branches for patching purposes, across areas normally owned by other SIGs on master?
Would an Architecture SIG own any code at all? How would it be decided that a PR needs to go through the Architecture SIG as well as the SIGs that own the files that the PR modifies? That sounds difficult to automate.

Would there be a default SIG that automatically owns all the code that's not owned by other SIGs?

Apologies if that's too many questions, too early. I certainly don't expect an answer to every one of them, but if there were some sample SIG layouts you could link here, I would be very interested to see how they're structured. That aside, I think having this slightly more formal code ownership mechanism is great, and it's definitely a net improvement over the current situation.

@dthaler dthaler self-requested a review April 1, 2020 22:04
Copy link
Contributor

@dthaler dthaler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had some wording suggestions, but nothing blocking and certainly better than before so noting Approval.

Copy link
Contributor

@jhand2 jhand2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to go through the docs and look for things that might need more discussion beyond the scope of this PR. Only found one in my opinion, but I welcome folks to look for others they want to flag.

docs/Community/Committers.md Outdated Show resolved Hide resolved

SIG-Attestation is responsible for code, plugins, testing, and other works
related to or required for attestation and integration with third-party /
external attestation services.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SIG-Attestation should also be responsible for tracking OESDK's alignment with attestation standards
(IETF RATS, TCG, etc.) and would like to see that stated explicitly. I personally spend a portion of my time doing just that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

@@ -0,0 +1,11 @@
chair:
- dthaler
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say someone else (maybe Hernan) would be better at this one

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed all the chair appointments from this pass, so that SIGs can properly self-organize :)

reviewers:

approvers:
- anakrish
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good if Intel had representation in SIG-SGX, or even as chair

Copy link
Contributor

@jhand2 jhand2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One suggestion might be to change the sig OWNERS files to be the minimal set of people we know should be approvers for each SIG (right now some SIGs include reviewers that don't work on those areas or this project anymore). We can add more people later as needed.

I would also be fine with merging this and removing people later, whatever people prefer

@AevaOnline
Copy link
Member Author

As per CGC meeting and agreement on April 24th, I have updated this PR and will now merge it. I am noting this since a self-approval is uncommon, and was explicitly requested in the meeting.

Additional changes e.g., to establish the SIG memberships and chairs, shall be done in subsequent PRs.

@AevaOnline
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Apr 27, 2020
2778: Restructure and update community governance docs r=AevaOnline a=AevaOnline

This is a first pass at re-organizing the community documentation. I anticipate a healthy conversation and several iterations of this work before it is acceptable to merge. It's a hefty patch, with substantial implications for the project, and I'd like folks to understand what I am suggesting before we merge this.

Changes:
- simplifying front page / README
- creating a docs/Community folder for documentation about community structure/governance
- refactored the top-level README to be a succinct summary of the project,
  with content mostly moved to docs/GettingStartedDocs/README
- added docs/Community/README, which outlines the community structure and helpfully links to other documents
- moved docs/Contributing to docs/Community/Contributing, clarified the
  "Design Discussion" section and added a "Weekly Issue Triage" section
- refactored docs/Governance into
    docs/Community/Governance  (which is also significantly expanded upon)
    docs/Community/ReleaseProcess  (mostly unchanged)
    docs/Community/conduct/ConflictPolicy  (mostly unchanged)
- refactored docs/Committers into
    docs/Community/Committers  (retains list of committers, though I'd like to remove this)
    docs/Community/governance/charter  (retains definition of scope)
    docs/Community/governance/README  (retains list of committee members)
- added a few SIG folders and templates as placeholders and examples
- added a few other files as placeholders for content that would be helpful to write soon

To get past the license check affecting documentation, I also added a
wildcard match in scripts.check-license.ignore for docs/*

Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>

Co-authored-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Co-authored-by: Aeva <806320+AevaOnline@users.noreply.github.com>
@bors
Copy link

bors bot commented Apr 27, 2020

Timed out.

@AevaOnline
Copy link
Member Author

bors retry

@bors
Copy link

bors bot commented Apr 27, 2020

Merge conflict.

AevaOnline and others added 14 commits April 27, 2020 12:16
This is a first pass at re-organizing the community documentation. I
anticipate a fair and healthy conversation, and several iterations of
this work before it is acceptable to merge. It's a hefty patch, with
substantial implications for the project, and I'd like folks to
understand what I am suggesting before we merge this.

Changes:
- simplifying front page / README
- creating a docs/Community folder for documentation about community structure/governance
- refactored the top-level README to be a succinct summary of the project,
  with content mostly moved to docs/GettingStartedDocs/README
- added docs/Community/README, which outlines the community structure and helpfully links to other documents
- moved docs/Contributing to docs/Community/Contributing, clarified the
  "Design Discussion" section and added a "Weekly Issue Triage" section
- refactored docs/Governance into
    docs/Community/Governance  (which is also significantly expanded upon)
    docs/Community/ReleaseProcess  (mostly unchanged)
    docs/Community/conduct/ConflictPolicy  (mostly unchanged)
- refactored docs/Committers into
    docs/Community/Committers  (retains list of committers)
    docs/Community/governance/charter  (retains definition of scope)
    docs/Community/governance/members  (retains list of committee members)
- added a few SIG folders and templates as placeholders
- added a few other files as placeholders for content that would be helpful to write soon

To get past the license check affecting documentation, I also added a
wildcard match in scripts.check-license.ignore for docs/*

Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Co-Authored-By: Amaury Chamayou <amaury@xargs.fr>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Co-Authored-By: Amaury Chamayou <amaury@xargs.fr>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Co-Authored-By: Amaury Chamayou <amaury@xargs.fr>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
adding minor fixes from reviews

Co-Authored-By: Simon Leet <31784195+CodeMonkeyLeet@users.noreply.github.com>
Co-Authored-By: Radhika Jandhyala <radhikaj@microsoft.com>
Co-Authored-By: Andy Schwartzmeyer <andrew@schwartzmeyer.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
- Add OWNERS files for several sigs, based on existing COMMITTERS file
- Add a template for sig charters
- Add a draft charter for sig-release, to test out the sig charter template
- rename "triage" to "wg-triage", as this feels more like a working group

Also includes some updates to Committers and Governance files related to
the establishment of clearer roles and processes for the creation of SIGs.

Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
minor fixes from reviews

Co-Authored-By: Simon Leet <31784195+CodeMonkeyLeet@users.noreply.github.com>
Co-Authored-By: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
Signed-off-by: Aeva Black <806320+AevaOnline@users.noreply.github.com>
@AevaOnline
Copy link
Member Author

bors retry

@bors
Copy link

bors bot commented Apr 27, 2020

Build succeeded:

@bors bors bot merged commit 2554287 into openenclave:master Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Tag indicating that an issue or PR needs review from a core OE developer process Tag indicating associated with a governance process. triaged This label classifies an issue/PR as having been triaged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants