-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
PIP-265: PR-based system for managing and reviewing PIPs #20207
Labels
Comments
Plan
|
Did we figure this part out?
I think it might make sense to keep the PIP type for a brief period with explicit instructions on how to follow the new process. |
@michaeljmarshall Check out the PR: #20418 |
Regarding, I'll modify the PIP issue template as well @michaeljmarshall |
This was referenced May 30, 2023
All done 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Background
In the last 2 months, I've increased my PIP review time (I do it in cycles), and reviewed quite a few PIPs.
My conclusion as a result of that:
It's nearly impossible to review PIPs using a mailing list.
We must fix it soon.
Why?
Very hard to review using email.
Let's say you review the PIP and find 10 issues. Once you quote and comment on those ten points, you basically started 10 threads of conversations.
After 2-3 ping pongs with quotes of quotes of quotes, it takes you forever to read each thread properly. You find yourself doing CTRL-F to search to find your original quote, and reply. Load it up again in your head, switching to the PIP tab to read it again.
After 10 ping pongs, it becomes almost an impossible mission.
I can say I'm 75% tired just from the process, not from the review itself.
It's non-collaborative by design.
After 10 ping pongs, the ability of someone to come and join the discussion is 0. They need to go through so many replies, which are half quotes, find the original reply, and browse to the PIP.
It's no wonder people drop off the PIP review once we cross 5–6 replies.
It's no wonder, nobody joins after 10 replies.
It's not open to the public. Non-collaborative.
You can't just get a link, and join the review. Not only because of (1) and (2). You need to join the mailing list. You don't get the past emails to reply. Just joining the list is a high enough bar for many people.
I personally participated in review of proposals in OpenTelemetry in the last 6 months, even though I'm just an occasional user. Why? They were conducted on GitHub PR, so it was easy for me - click a link and reply.
All over the place
Sometimes people comment on the GitHub issue.
Sometimes on the mailing list.
Not a single place.
No history.
Ok, finally the author was convinced. I can't see just the changes. They need to explicitly tell me something was changed.
Delete All.
They can go crazy, after 1 year, edit the GitHub issue, delete all the text and write "Kafka is the king". No history, nobody can stop them. It's their issue.
Show me all the approved PIPs
Hard to track it today, hard to keep up to date.
Resolved comments
Even though you managed to read all 35 replies so far, in reply 36 you see the author agreed to all 8 out of 10 suggestions. You have no idea of knowing that in advance. You just wasted 1 hour.
Proposal
PR is the main tool we have that allows multiple threaded discussion.
Git provides history. You can't delete it without approval from PMC members.
We'll create a folder named "pip" in the pulsar main repo. It will contain one markdown file for each PIP. The file will be named
pip-xxx.md
. We will write below how to obtainxxx
before you start.To create a PIP, you grab
pip/template.md
and use it to compose your file in thepip
folder.You submit this file as a PR named "PIP-xxx: {short description}".
You create "[DISCUSS] PIP-xxx: {short description}" in the DEV mailing list and refer people to your PR, with short text explaining the gist of it.
People discuss using PR comments, each is its own-threaded comment. General comments can be made both as replies in the mailing list or as general comment in the PR. After 10 PIPs in this way we’ll be able to see what people gravitate towards and what’s more convenient and consider refining that.
Comment was done? They resolve it. This way you see what the pending discussions are at a glance.
Reached consensus? Good. Send "[VOTE] PIP-xxx: short description" on DEV mailing list,
Following vote process as described before:
Everyone is welcome to vote on the proposal, though only the vote of the PMC members will be considered binding. It is required to have a lazy majority of at least 3 binding +1s votes. The vote should stay open for at least 48 hours.
PIP approved? Awesome. Push commit with link to the vote on mailing list.
A PMC member will merge it.
Merge == approved.
PMC members can add a PIP label.
Rejected? All good. Close the PR.
Closed == Rejected.
It can't be deleted. All comments are still there.
There will be README in the
pip
folder containing:This will replace existing
wiki/proposal/PIP.md
which will be merged into the README.All links pointing to that
PIP.md
file (from wiki, pulsar-site) will be amended to point to the new README file.Grabbing PIP number
Before you start, you search Pull Requests with label PIP in GitHub (
is:pr "PIP-" in:title
)Take the biggest number and add 1.
It is super rare to have two people create PR at the same time.
Show me all approved PIPs
Option 1: Search merged PRs labeled PIP.
Option 2: Look at "pip" folder
Show me rejected PIPs
Search closed PRs with "PIP-" in title, or labeled PIP.
Handling images
Since documents will now reside as files in git, we need to avoid large image files.
Hence, we'll specify to author that images needs to be created using mermaidJS diagram language, which GitHub supports rendering. It covers 99% of the cases. For the 1% case, they can use small file size format SVG, and make sure the file is 1k-5k size.
Alternatives
Use PR for voting instead of mailing list
Processed are best changes one step at a time. This can be considered in the future, but out of scope for this proposal. For now, we’ll keep the voting as it is today: in the VOTE mail in the DEV mailing list.
General discussion should be only in the mailing list
Judging from PIPs I read, I would say the majority of the feedback is not in the scope of the PIP, but in the scope of certain section / part of the PIP. If 90% of the comments already transpire in the PR, I don’t think it will benefit for the mere 10%.
Also, human beings are hard at using two systems at the same time :)
Another big advantage on keeping the general comments in the PR is that it’s public and everybody can pitch in - For example, Eron Wright was invited to help on a PIP for Open ID Connect (Michael) by team members. If the barrier was joining the mailing list, we might not get that valuable comment.
Yet, I think it’s best to do changes incrementally, and use both the mailing list and PR and see what people choose organically.
A separate repo for “pips”
It’s possible, of course, but I fear the following:
We can always move it if it really causes a pain point to many people.
Keeping a README with file name and proposal description
We have that today, in the wiki, but as can be seen, people forget to update it. We can always add such a list in the future. Today the list is the files in the directory and link to reject PIPs (Search of closed PRs with label of PIP)
Links
Discussion leading to this PIP: https://lists.apache.org/thread/5kpddlfh5xdbsjmv47ymnk3z6wd92jbh
DEV email: https://lists.apache.org/thread/2624hnox7kb7p4k5qhfpr4rkt6yh5jp1
VOTE email: https://lists.apache.org/thread/lsojtpc4s86omwtly1gxczs3tbbd0wnz
The text was updated successfully, but these errors were encountered: