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

PR template with some guidance #770

Merged
merged 2 commits into from
Jan 4, 2024
Merged

PR template with some guidance #770

merged 2 commits into from
Jan 4, 2024

Conversation

anorth
Copy link
Member

@anorth anorth commented Jul 30, 2023

This PR proposed a PR template, which GitHub will auto-fill in the description field of any new PRs (docs). It contains some guidance that I've frequently found myself giving FIP authors after they have already submitted their PR.

@filecoin-project/fips-editors I'd welcome thoughts about anything else that we should say here.

Please open a new discussion topic in the FIPs repository discussion board and outline you motivation and proposal,
then come back in a few days or weeks to submit your draft FIP.

Don't just paste your FIP draft into a discussion topic.
Copy link
Member

@raulk raulk Jul 31, 2023

Choose a reason for hiding this comment

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

My two cents here.

  • Holding conversation within a FIP PR and within a discussion thread tends to splinter the debate around a certain topic (based on my observations).
  • I'd rather throw away any process related to discussions. Whether it's obliging certain discussions to exist, or managing discussions in a certain manner. The more process exists, the more we stifle fluidity, spontaneity, creativity, and fun. Discussions should be a free flowing space, with a low bar (and templateless).
  • It's up to the author to decide how much they want to invest in an idea before they submit it to the community.
    • I expect authors to display enough restraint to not overdevelop an idea and get married to an outcome before presenting it to the community for the first time.
    • I expect authors to display enough emotional regulation if the community shows aversion against their idea, no matter how much time they've invested in it.
    • (sidenote: acknowledging that controversial ideas require complex consensus and often result in fuzzy outcomes)
  • Perhaps we should code author expectations, retain their agency, trust they will do the right thing, and hold them accountable to that standard.
  • Some early ideas are truly better submitted as draft FIPs, especifically normative/technical ones (e.g. FVM ones, gas changes, syscall changes, event schema changes). Some authors prefer to submit early ideas structrured as per the standard template (FIP, in this case), so as to minimise future standard development.
  • I would personally like to undo the policy that a discussion thread must exist for every FIP.
    • If the author deems prior discussion necessary, they're welcome to create it.
    • If the author is more action-oriented and wants to submit a minimal FIP first, they're welcome to do so, as long as there's sufficient evidence of initial analysis, ponderation, conceptualisation, impact analysis, etc. The FIP can continue maturing as discussion progresses.
      • I'd instate the convention that "general" discussion happens on a comment thread on line 1 (title), OR as review comments (folks can comment on a PR without inlining comments).
    • If a discussion transitions into a FIP, I'd expect the FIP to reference the original discussion, and encode much of the content in the FIP's body.

Note that I personally bias towards encoding including as much history, alternative views, counterarguments, etc. arising in a discussion in the FIP's text to assure survival and avoid platform dependence. GitHub Discussions are a proprietary non-portable platform, whereas Markdown texts are portable and open.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi! Good points, overall. I don't particularly disagree.

The original reason for requiring a discussion post (and, as is the case for most hard-and-fast rules we try and maintain) is because of the nature of our soft consensus acceptance protocol. Without a dedicated space for community discussion, it is difficult to assess whether or not a FIP has actually completed its 'Last Call' period. It is also a single space we can use to track community conversations around really contentious FIPs; when soft consensus is the norm, we need a single point-of-reference for capturing feedback.

However, as we try and move away from this idea of ONLY accepting FIPs via a soft consensus community protocol, I'm very open to the idea that discussion posts should not be required.

My only real concern is that by not requiring a discussion post, we'll see a drastic fall off in open, accessible community discussions around FIPs-in-progress. Yes, process can be stifling, but this isn't a particularly hard line; in the case where authors have opened quality PRs without discussion posts, we simply ask them to open one. Imo, we could certainly try removing the requirement for discussion posts (after we, hopefully, overhaul certain parts of FIP0001), but I personally would rather police an extra rule than see our current conversation threads completely exit a community forum.

Copy link
Member

Choose a reason for hiding this comment

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

I understand the concern but also don't think PRs are a great forum for discussion, even with the comment on line 1 trick. I'm yet to see any big issues/burdens surrounding the existence of a separate discussion, even when this is created after FIP submission (see #803 & related discussion #844 that came after yet still recorded activity).

I am, however, inclined to agree with @raulk that we don't need to impose a waiting period between discussion and PR.

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've removed the text here about any delay, or even sequencing, between discussion and FIP draft.

.github/pull_request_template.md Outdated Show resolved Hide resolved
@kaitlin-beegle
Copy link
Collaborator

A small change I'd like to propose is adding a Consensus Decision to the FIPs template. This was first suggested by @jsoares (see: HERE) and I think it's a good idea.

If there's positive sentiment about doing this, I'll make the change on the file.

Also, fwiw, the open FIP0001v2 proposal suggests new FIP categories- at this time, I don't think there's a need for FIPs of different times to have different templates. This could change, but I think one standard template is currently appropriate for FIPs of different types.

@anorth anorth force-pushed the anorth/pr-template branch 2 times, most recently from f247a00 to e3c94a1 Compare October 25, 2023 19:47
@anorth
Copy link
Member Author

anorth commented Oct 25, 2023

A small change I'd like to propose is adding a Consensus Decision to the FIPs template.

Done

I don't think there's a need for FIPs of different times to have different templates

I've removed the confusing "_FTP" suffix from the FIP template file.

@jennijuju
Copy link
Member

jennijuju commented Jan 3, 2024

A small change I'd like to propose is adding a Consensus Decision to the FIPs template.
Done

I prefer to add the consensus decision session AFTER FIP0001v2 is approved. For the time being it serves the same role has the fip status (either passed or not passed by soft consensus)

otherwise the idea and the list LGTM, thank you for adding!

@anorth
Copy link
Member Author

anorth commented Jan 4, 2024

I prefer to add the consensus decision session AFTER FIP0001v2 is approved. For the time being it serves the same role has the fip status (either passed or not passed by soft consensus)

Fair, I've removed it so I can land the obvious parts.

@anorth anorth merged commit 71e8465 into master Jan 4, 2024
1 check passed
@anorth anorth deleted the anorth/pr-template branch January 4, 2024 22:36
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.

None yet

5 participants