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

Update EIP-5069: Replace handbook with charter #7468

Merged
merged 8 commits into from
Aug 23, 2023

Conversation

SamWilsn
Copy link
Contributor

@SamWilsn SamWilsn commented Aug 9, 2023

No description provided.

@SamWilsn SamWilsn requested a review from eth-bot as a code owner August 9, 2023 15:05
@github-actions github-actions bot added c-update Modifies an existing proposal t-process labels Aug 9, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Aug 9, 2023

🛑 Auto merge failed. Please see logs for more details, and report this issue at the eip-review-bot repository.

@eth-bot eth-bot changed the title Replace handbook with charter Update EIP-5069: Replace handbook with charter Aug 9, 2023
@eth-bot eth-bot enabled auto-merge (squash) August 9, 2023 15:06
eth-bot
eth-bot previously approved these changes Aug 9, 2023
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@SamWilsn SamWilsn marked this pull request as draft August 9, 2023 15:07
auto-merge was automatically disabled August 9, 2023 15:07

Pull request was converted to draft

EIPS/eip-5069.md Outdated Show resolved Hide resolved
g11tech
g11tech previously approved these changes Aug 17, 2023
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

EIPS/eip-5069.md Outdated Show resolved Hide resolved
EIPS/eip-5069.md Outdated Show resolved Hide resolved
EIPS/eip-5069.md Outdated Show resolved Hide resolved
EIPS/eip-5069.md Outdated Show resolved Hide resolved
EIPS/eip-5069.md Outdated Show resolved Hide resolved
@SamWilsn
Copy link
Contributor Author

Two comments from @gcolvin:

[W]e don't generally need anyone to call consensus unless someone has a blocking objection, and even then we hope to resolve the blocking issue . Deciding to move on despite the block is a last resort. When it really is necessary having an Editor make the call can put that person in a difficult position. I said on the call that if [@poojaranjan] is willing she would be a better neutral arbitrator. [...] These aren't blocking objections for me, but I don't want to approve it as it is.

[Replace Janitor with Moderator o]r some title other than Janitor. Their duties are not at all like mopping floors and scrubbing sinks.

@xinbenlv
Copy link
Contributor

Thanks for the effort! I am very encouraged that this could reduce a lot of friction and boost productivity of editing!

A few feedback:

    1. I am quite a big fan of making it explicit "we don't decide winner", kudos for spell it out even more clearly, @SamWilsn 👍
    1. the current version seems to establish a new role "the janitor" and a power "to interpret consensus", which seems an important decision. 🔢

After thirty days from the call for input, if not all EIP Editors have responded, the quorum is reduced to the Editors that have responded. This deadline may be extended in exceptional situations.

This means the editors need to be able to properly track responses from others. What's our way to ensure such response.
Shall we define a venue / medium to recognize as legitimate response? i.e. How do we avoid the confusion of

  • "oh, I msged this channel"
  • "I spoke on Zoom meeting"
  • "oh I responded on that Github issue"
  • "oh I msg someone in DM" etc

For example, Apache requires all important conversation occurs in Emails so there is a integrated way to notify everyone.

What's our preferred practice?

EIPS/eip-5069.md Outdated Show resolved Hide resolved
EIPS/eip-5069.md Outdated Show resolved Hide resolved
EIPS/eip-5069.md Outdated Show resolved Hide resolved
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

I feel like there are some roles missing. Here is what I think needs specifying:

  • Janitor/Moderator/<namehere>: Determining when consensus has been reached
  • Governing Editor: Participates in governance, voting, and elections
  • EIP Repo admin: Greater moderation powers; ability to make major configuration changes
  • EIP Repo write: Lesser moderation powers; ability to make minor configuration changes
  • EIP Editor: Reviews proposals. Is not necessarily a governing editor

There are a few editor-adjacent roles that have powers that also need specifying:

  • eipw/eipw-action maintainers (that's currently @SamWilsn)
  • eip-review-bot (and any other bots that interact using @eth-bot) maintainers (that's currently me, @Pandapip1)
  • Website maintainers (me & @SamWilsn)
  • Other CI maintainers (me & @SamWilsn)

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
@SamWilsn
Copy link
Contributor Author

This means the editors need to be able to properly track responses from others. What's our way to ensure such response. Shall we define a venue / medium to recognize as legitimate response? i.e. How do we avoid the confusion of [where to post]? What's our preferred practice?

That's a great question @xinbenlv. I've tried to avoid implementation details as much as possible within this document, because I think they'll end up changing quite frequently.

This particular issue falls directly under the Janitor's duty to "determine when rough consensus has been reached", so wherever the Janitor says opinions/votes have to be registered is where they have to be registered. If the Janitor is unreasonable, vote 'em out.

@SamWilsn
Copy link
Contributor Author

SamWilsn commented Aug 19, 2023

I feel like there are some roles missing. Here is what I think needs specifying:

I'd rather make an error by under-specifying than by over-specifying.

  • Janitor/Moderator/<namehere>: Determining when consensus has been reached

We've got this one ;)

  • Governing Editor: Participates in governance, voting, and elections

So this would be "EIP Editor" as written today?

  • EIP Repo admin: Greater moderation powers; ability to make major configuration changes
  • EIP Repo write: Lesser moderation powers; ability to make minor configuration changes

Hm... These feel too specific to GitHub and are maybe an implementation detail? If the Editors with admin rights misbehave, they can be removed as Editors. I'd love to grant "delete an issue" rights but there's something funky about our GitHub organization that prevents it (or so I've heard.)

  • EIP Editor: Reviews proposals. Is not necessarily a governing editor

So this would be someone who counts for approving PRs, but doesn't get to express opinions/votes for governance purposes?

Does the idea of Topic Groups (see fb4a1a0) work for this? Or are you talking specifically approve PRs that only EIP Editors can approve?

Does the delegation of powers paragraph cover this? An Editor could delegate their PR approving powers to anyone they choose.

There are a few editor-adjacent roles that have powers that also need specifying:

  • eipw/eipw-action maintainers (that's currently @SamWilsn)
  • eip-review-bot (and any other bots that interact using @eth-bot) maintainers (that's currently me, @Pandapip1)
  • Website maintainers (me & @SamWilsn)
  • Other CI maintainers (me & @SamWilsn)

Do these need to be called out in our organizational charter, or is this more like "If you have questions about this bot, talk to @Pandapip1"?

@Pandapip1
Copy link
Member

I guess a lot of this can be covered by delegation. While that's not ideal, I won't block for it.

Maybe a seperate document (or documents) detailing implementation-specific powers could be useful?

g11tech
g11tech previously approved these changes Aug 23, 2023
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

@SamWilsn SamWilsn marked this pull request as ready for review August 23, 2023 14:53
@SamWilsn SamWilsn merged commit b5b2c8d into ethereum:master Aug 23, 2023
15 of 17 checks passed
@SamWilsn SamWilsn deleted the charter-1 branch August 23, 2023 14:55
streamnft-tech pushed a commit to streamnft-tech/EIPs that referenced this pull request Oct 27, 2023
* Replace handbook with charter

* Remove topic groups

* Small grammar fix

* Apply @Pandapip1's suggestions

* Remove extra space

* Update EIPS/eip-5069.md

Co-authored-by: Gavin John <gavinnjohn@gmail.com>

* s/Janitor/Keeper

* Update eip-5069.md

---------

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
RaphaelHardFork pushed a commit to RaphaelHardFork/EIPs that referenced this pull request Jan 30, 2024
* Replace handbook with charter

* Remove topic groups

* Small grammar fix

* Apply @Pandapip1's suggestions

* Remove extra space

* Update EIPS/eip-5069.md

Co-authored-by: Gavin John <gavinnjohn@gmail.com>

* s/Janitor/Keeper

* Update eip-5069.md

---------

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
just-a-node pushed a commit to connext/EIPs that referenced this pull request Feb 17, 2024
* Replace handbook with charter

* Remove topic groups

* Small grammar fix

* Apply @Pandapip1's suggestions

* Remove extra space

* Update EIPS/eip-5069.md

Co-authored-by: Gavin John <gavinnjohn@gmail.com>

* s/Janitor/Keeper

* Update eip-5069.md

---------

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
GAEAlimited pushed a commit to GAEAlimited/EIPs that referenced this pull request Jun 19, 2024
* Replace handbook with charter

* Remove topic groups

* Small grammar fix

* Apply @Pandapip1's suggestions

* Remove extra space

* Update EIPS/eip-5069.md

Co-authored-by: Gavin John <gavinnjohn@gmail.com>

* s/Janitor/Keeper

* Update eip-5069.md

---------

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
blacksnow2 pushed a commit to blacksnow2/EIPs that referenced this pull request Jul 21, 2024
* Replace handbook with charter

* Remove topic groups

* Small grammar fix

* Apply @Pandapip1's suggestions

* Remove extra space

* Update EIPS/eip-5069.md

Co-authored-by: Gavin John <gavinnjohn@gmail.com>

* s/Janitor/Keeper

* Update eip-5069.md

---------

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal t-process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants