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

SDK governance draft #649

Merged
merged 7 commits into from
Jul 2, 2020
Merged

Conversation

slinkydeveloper
Copy link
Member

@slinkydeveloper slinkydeveloper commented Jun 24, 2020

As discussed in CloudEvents WG Meeting June 18, 2020, I started drafting some bits about the sdk projects governance, in particular regards:

  • Keeping the projects healthy
  • Handling situations where maintainers are not active
  • Voting process

This PR doesn't cover the add/remove maintainers policies, which IMHO deserves a separate discussion, but the proposed document is meant to host them (I've added a blank section for them)

All the numbers are drafted, I focused mainly on the criteria and the process.

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
Copy link
Member

@lance lance left a comment

Choose a reason for hiding this comment

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

Lots of little nits and a few larger suggested changes. Thanks for putting this together!

We define an SDK project _healthy_ if:

1. It works with the latest version of the programming language
2. It supports the latest versions of the integrated libraries/frameworks
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean "dependencies are kept up to date"? If so, why not just say 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.

Does this mean "dependencies are kept up to date"? If so, why not just say that?

That's not necessary true, in ecosystems like Java libraries could be bytecode backward compatibility. For example, if sdk-java depends on library-x v1.0, it could work with v1.1 or even with v2.0 without touching sdk-java.

Copy link
Member

Choose a reason for hiding this comment

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

So I guess I'm just not clear on what this means.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure how i could rephrase it honestly 🤷‍♂️


1. It works with the latest version of the programming language
2. It supports the latest versions of the integrated libraries/frameworks
3. It receives security patches regularly
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
3. It receives security patches regularly
3. Any security patches received are applied and the library is published regularly

This phrasing is a little odd. What if there are no security issues with the library? Slightly modified to address 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.

If there are no security issues and all the other criteria are respected, then there is no need to publish regularly.
Your phrasing seems like saying "even if you meet 1, 2 and there is no need of security patches, library should be published regularly" which doesn't make sense to me honestly. If the library works, it's not buggy, it's secure and new 3rd party integrations can interact with it, there is no need to publish it.

Copy link
Member

Choose a reason for hiding this comment

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

OK, how about this?

3. Any security patches received are applied and published within one month of receipt

Copy link
Member Author

Choose a reason for hiding this comment

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

but then it seems similar to criteria 8... Maybe should I remove one of the two?

governance/SDK.md Outdated Show resolved Hide resolved
Comment on lines +42 to +43
- Temporary security patches delivered by an _sdk maintainer_ not part of
`sdk-x-maintainers` group
Copy link
Member

Choose a reason for hiding this comment

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

This sentence is a bit confusing. Two things stand out. First, what is an sdk maintainer? Is this someone who is in the sdk maintainers group but who is not a maintainer on X repository? Second, if so, how does this remedy the situation where a security patch is not released? Do the sdk maintainers (union of all sdk-x-maintainers) each have publish credentials for all SDKs? That seems insecure.

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 tried to explain it in the first section of the document the difference between sdk maintainer and sdk-x-maintainer, how could i reword this sentence?

About having the rights to publish release, we had a discussion a while ago with @n3wscott which proposed to automate it, in order to avoid a single person to hold rights to publish packages. The part of this document about security patches kinda "assumes" that we have such automation in place

governance/SDK.md Outdated Show resolved Hide resolved
- A member of the community entitled to manage the vote, called voting manager.
The voting manager MUST also be part of people entitled to vote.

To start a vote, the voting manager opens an issue in the
Copy link
Member

Choose a reason for hiding this comment

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

Who is the "voting manager"?

Copy link
Member Author

Choose a reason for hiding this comment

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

explained in the row above, let me try to rephrase it

Comment on lines +158 to +160
considered; To calculate the agreement percentage, the voting manager should
account the people who have expressed a vote, without taking in account the
abstentions.
Copy link
Member

Choose a reason for hiding this comment

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

This section is unclear to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's about taking the abstentions out of the final count of votes. Let's take as example:

  • 8 people are entitled to vote
  • Required 50% + 1 for the vote to pass
  • 3 effectively votes, the others don't
  • 2 -1, 1 +1

Then the vote does not pass, because less than 50%+1 voted +1

governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@slinkydeveloper
Copy link
Member Author

This PR assumes that every sdk has a release automation which doesn't require more than the github write permissions to perform a release. Should i write it in the doc?

governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
[Contribution Acceptance](../SDK.md#contribution-acceptance)
5. Issues and PRs are triaged (labeled, commented, reviewed, etc) regularly

We define a project `cloudevents/sdk-x` _not actively maintained_ if:
Copy link
Collaborator

@duglin duglin Jun 28, 2020

Choose a reason for hiding this comment

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

What if an SDK has no activity because it doesn't need any work done on it? No language updates, no CE updates, no issues, etc... it's just working.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then It's fine, I don't define any actions in such cases. I define actions when issues/PR are not being triaged (archive/handover the project) or if the security patches are not applied and released.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe should i remove the criteria 6?

Copy link
Collaborator

@duglin duglin Jun 29, 2020

Choose a reason for hiding this comment

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

Yep, number 6 is the one that got me thinking about this... I agree we need something, just not sure how to word it.
I wonder if instead of being too prescriptive, it should be a bit more vague to allow for the other sdk maintainers to use their best judgement. Something like:

There has been no necessary activity within the SDK for at least 4 months. The other SDK maintainers will
evaluate what "no necessary activity" means, but often this could mean "no commits", or "no issue
discussions". However, if the SDK is stable and does not need to be update then it might be determined
that it current state is acceptable.

I don't know...

Copy link
Member Author

@slinkydeveloper slinkydeveloper Jun 30, 2020

Choose a reason for hiding this comment

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

I like it, but I prefer to keep it as note more than as criteria, because it's ok to have it vague but it's too much open to "interpretations" to be a criteria. let me try to reword it

maintainers. The community can perform the handover to a new maintainer
if all the following conditions are met:

* A vote to add the new maintainer MUST have **already taken place** with
Copy link
Collaborator

Choose a reason for hiding this comment

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

Who initiates a vote if all maintainers have vanished?

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, I guess it's implied that it's one of the total sdk maintainers.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's explained below

A voting manager, a member of the community entitled to manage the vote. The voting manager MUST be part of people entitled to vote.

governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
governance/SDK.md Outdated Show resolved Hide resolved
does not pass, then a new vote for the same change MUST NOT happen for the
next 6 months.

This voting process MUST be used only to resolve the issues identified in this
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if this was the intent, but are you saying that this voting process can NOT be used for other topics? I'm wondering if you meant to say:
This voting process must be used in the following cases, but can be used in others as well:

Copy link
Member Author

Choose a reason for hiding this comment

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

My intent is to avoid people starting abusing of this (heavy) process for things that doesn't really need it.
If we want to add in future use cases where we want to use this process, then it's fine, we just need to change this document and agree on the changes

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@duglin
Copy link
Collaborator

duglin commented Jul 2, 2020

Approved on the 7/2 call

@duglin duglin merged commit 4ea37b6 into cloudevents:master Jul 2, 2020
@slinkydeveloper slinkydeveloper deleted the governance branch July 9, 2020 09:57
n3wscott pushed a commit to n3wscott/cloudevents-spec that referenced this pull request Nov 30, 2020
* Draft of SDK governance

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Nit on security patches

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Formatted

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* CI

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* First pass

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Update and refactored

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Removed criteria about commits on master

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@duglin duglin mentioned this pull request Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants