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 Committers section of CONTRIBUTING.md #23685

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tdcmeehan
Copy link
Contributor

Description

Describe the contributor ladder to better describe the various roles of the project.

Motivation and Context

Better description of the roles of the project and how to move between them.

Impact

Better documentation.

Test Plan

N/A

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

== NO RELEASE NOTE ==

Describe the contributor ladder to better describe the various
roles of the project.
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Only a few minor nits and suggestions to consider.


### Baseline expectations from committers
Contributors are community members who actively participate by submitting pull requests (PRs), engaging in discussions,
and reviewing code. They are the foundation of the Presto community. There are no requirements to become a Contributor,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
and reviewing code. They are the foundation of the Presto community. There are no requirements to become a Contributor,
and reviewing code. They are the foundation of the Presto community.

I had cognitive dissonance from a list of "Requirements" immediately following the statement "There are no requirements to become a Contributor". I think the second half of this sentence "anyone can contribute to the project by following the guidelines outlined in this document" presents the openness of the role.

### Baseline expectations from committers
Contributors are community members who actively participate by submitting pull requests (PRs), engaging in discussions,
and reviewing code. They are the foundation of the Presto community. There are no requirements to become a Contributor,
and anyone can contribute to the project by following the guidelines outlined in this document.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
and anyone can contribute to the project by following the guidelines outlined in this document.
Anyone can contribute to the project by following the guidelines outlined in this document.


Steps to become a Committer:

1. Prepare a document outlining your contributions to Presto, including your GitHub stats (number of reviews, lines of code added, number of PRs, etc.).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. Prepare a document outlining your contributions to Presto, including your GitHub stats (number of reviews, lines of code added, number of PRs, etc.).
1. Prepare a document outlining your contributions to Presto and share it with a TSC member. Include your GitHub stats such as number of reviews, lines of code added, number of PRs, and other stats.

Nits: avoid etc., and avoid parentheses as they slow down reading comprehension. Broke into two shorter sentences to help readability.

I added " and share it with a TSC member" to connect to the next item. If there is anything more formal or detailed about how to share the doc that you want to describe, please add whatever detail you think needed. If any additional detail, suggest adding only the minimum amount necessary.


1. Prepare a document outlining your contributions to Presto, including your GitHub stats (number of reviews, lines of code added, number of PRs, etc.).
2. If a TSC member believes you are eligible, they will nominate you for a vote.
3. The TSC will vote on your nomination, and if you receive a majority approval, a PR will be raised to add your name to the [`CODEOWNERS`](CODEOWNERS) file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
3. The TSC will vote on your nomination, and if you receive a majority approval, a PR will be raised to add your name to the [`CODEOWNERS`](CODEOWNERS) file.
3. When the TSC votes on your nomination, if you receive a majority approval a PR will be opened to add your name to the [`CODEOWNERS`](CODEOWNERS) file.

Minor revision suggestion for flow from the previous item.

Copy link
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

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

This is a clear improvement on the nature of the different committer types, which I found confusing in the old docs.

I do think we're still process heavy for a project of this size, and I'd like to be careful not to add any new rules in this document that we haven't previously committed to or aren't already following.


The Presto community has four main roles: Contributor, Module Committer, Project Committer, and Technical Steering Committee (TSC) Member.
Each role has specific requirements, responsibilities, and benefits. The roles are designed to recognize and reward community members who
Copy link
Contributor

Choose a reason for hiding this comment

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

"The roles are designed to recognize and reward community members who
maintain the project."

This doesn't sound right to me. The roles are jobs and duties, not rewards or medals. I suggest dropping this sentence.

* Recognition in release notes and community updates.
* Influence the project’s direction through active contributions and participation.
* Opportunity to advance to more senior roles such as Module Committer or Project Committer.
* Follow the project’s guidelines and values, including those outlined in this document.
Copy link
Contributor

Choose a reason for hiding this comment

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

The last bullet point doesn't feel like a benefit


#### Expectations of a module committer
Benefits:
Copy link
Contributor

Choose a reason for hiding this comment

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

I might drop this section. It feels fluffy.


### Project committers
* Demonstrate technical mastery of a specific module or area of the codebase.
* Consistent, high-quality contributions and reviews to the module over an extended period (usually 6+ months).
Copy link
Contributor

Choose a reason for hiding this comment

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

what do we do about someone who introduces a new module, typically a connector for a new backend, that no committer knows anything about?

* They have experience with reviewing code and making code changes outside of their core area of expertise;
* They set a high bar for their own contributions and those of others during code reviews, including avoiding hacks and temporary workarounds;
* They go above and beyond a module committer in helping maintain the project by regularly reviewing code outside of their area of expertise, or helping users of the project in public channels such as Slack, GitHub, or helping review designs outside of their area of expertise such as providing guidance on Github Issues or RFCs.
Benefits:
Copy link
Contributor

Choose a reason for hiding this comment

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

again, this feels fluffy


### Voting for committers
Project Committers have broad authority across the entire Presto codebase and are recognized as core contributors to the
project . They are responsible for ensuring the quality and direction of the project as a whole, and they have access to
Copy link
Contributor

Choose a reason for hiding this comment

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

extra space before period


* Long-term, sustained contributions to the project.
* Deep involvement in key project decisions and strategic planning.
* Experience as a Project Committer with demonstrated leadership.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I'm comfortable with making Project committer a requirement for TSC member. I think this should be broader.

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