-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Presto Open Source Community Roles & Responsibilities
Rongrong Zhong edited this page Mar 8, 2019
·
3 revisions
This wiki page lays out the responsibilities of the different roles that participate in the development of the Presto open source project.
A reviewer is someone who reviews code for the Presto open source project. The reviewer doesn’t need to be a committer, anyone in the community is welcome to review code for the project.
- Provide high quality reviews/feedback conforming to Presto coding style & best practices.
- Work with the PR (pull request) author to make sure that the change under review doesn't regress in terms of functionality, performance, and reliability.
- If help is needed during code review for certain parts of the PR, ask for help & bring other reviewer(s) to the PR.
- A reviewer may revert a particular change if the change breaks functionality, causes a performance or reliability regression, or is otherwise problematic. The reviewer should then work with the author of the PR to fix the issue.
- Provide unambiguous comments and directions - reviewers should call out comments as nits/nice-to-have's/suggestions/blockers, or explicitly defer final review to other reviewers. If a reviewer “request changes” on GitHub, that usually means the reviewer wants to do another review after the changes are made.
- Reviewers, if they are unavailable or don't have sufficient expertise, can delegate to other reviewers (or contributors) for reviewing a particular PR.
- Reviewers should provide an initial feedback as soon as possible (preferably under 2 days). The initial feedback can be about the direction of the PR, request for changes, asking for more time (e.g., I cannot review your PR now, but I will definitely get back to it at X time), or propose another reviewer.
A contributor is someone who contributes code to the Presto open source project.
- When opening a PR the author should provide enough details, context, motivation, etc. about the change. For large/architectural changes, it is preferred to have a lightweight design document in the form of a GitHub issue or Google doc with the description of the problem statement and proposed solution (possibly including other/simpler solutions considered), rather than starting with a large amount of code. In some cases, it may be best to first begin with a proof-of-concept PR to quickly validate direction. This same guidance applies to syntax changes (e.g., “syntax-needs-review” label).
- PRs must be written in a manner that makes the review process as simple as possible. This includes, but is not limited to:
- Preferring small, incremental changes relative to big changes.
- Using feature flags (config and session properties) where appropriate.
- Constructing large PRs as a logical series of smaller commits, each of which compile and pass tests.
- Carefully considering API changes and avoiding new APIs wherever possible.
- Sticking to ANSI SQL language specification wherever possible, and providing context about similar functionality in other industry-standard engines (e.g. referencing PostgreSQL function documentation).
- Attempting to identify the most experienced people in that area of code as the first reviewer.
- Providing documentation and release notes as part of the change, when appropriate.
- Code should conform to Presto style guides and best practices.
- Commit messages should conform to Presto commit message guidelines.
- Code should have proper unit and integration tests (if necessary).
- For changes around performance critical code the change should have benchmarks (JMH or otherwise).
- The author should be responsive to reviewers' feedback/comments and address them.
- If the reviewer is not responsive the PR author can ping the reviewer and can also bring in other reviewers if needed.
- If the author needs certain people to review certain parts of the PR the author should let them know.
- Sometimes it's easier to resolve issues (either in PRs or in production) through synchronous, direct conversations. Authors should try to be online on slack and have those discussions if necessary.
- It would be great if the contributors can also help the Presto open source community by answering questions about the features they have contributed when the users ask questions about these features on Slack or Google groups.
A committer is someone who can check in code to the Presto source code repository. How to become a committer is described in this document. Here are the responsibilities of Presto committers:
- Committers should help reviewing pull requests to make sure that they are high quality, correct, and do not regress functionality, performance, and reliability.
- Committers should contribute to design/technical discussions on Github and Slack.
- Committers should help supporting the community through answering questions on Github, Slack, and the user mailing list.
- Committers can nominate other contributors as committers if they meet the requirements outlined in this document.
- Committers should have good judgement around when some code should be merged, or when to ask someone else to make that judgement.
- Committers should help maintain the high quality of our code base, tests, and our documentation.