aip-1: Proposal Process Draft PR #3
Conversation
|
this is related to #1 |
proposals/aip-1.md
Outdated
| --- | ||
|
|
||
| # What is an Agorex Improvement Proposal (AIP)? # | ||
| An AIP is a document providing information to the Agorex community or describing process that the Agorex community follows. The AIP should provide a concise specification of a process and a rationale for it. The AIP author is responsible for building consensus within the community and documenting dissenting opinions. |
There was a problem hiding this comment.
This paragraph is a little confusing. I would change it to:
"An AIP is a document providing information to the Agorex community that describes a new process or feature within the organizational structure or the exchange itself. The AIP should provide a concise specification of the feature or process and a rationale for the feature. The AIP author is responsible for building consensus within the community and documenting dissenting opinions."
There was a problem hiding this comment.
@JonathonDunford @rowmagnon Thank you for the feedback. I'm fine with this revision. However, I wonder if we want to include "feature" at all though (or at least maybe not now). I found the EIP wording to be fairly unclear for processes. It appears to me the doc was written for features and as an afterthought processes/meta was worked in. I have been thinking of optimizing the wording of AIP-1 to be "process" specific (and toyed with wording of "policy" too). I think feature decisions generally don't require the formality of an AIP and maybe we should focus on processes/policies instead for now. Then later we could add in features if we find that helpful. What are your thoughts?
proposals/aip-1.md
Outdated
| An AIP is a document providing information to the Agorex community or describing process that the Agorex community follows. The AIP should provide a concise specification of a process and a rationale for it. The AIP author is responsible for building consensus within the community and documenting dissenting opinions. | ||
|
|
||
|
|
||
| # Rational # |
|
|
||
| - A **Process Proposal** describes a process surrounding Agorex or proposes a change to (or an event in) a process. Process Proposals often require community consensus and unlike Informational Proposals, they are more than recommendations, and users are typically not free to ignore them. Examples include procedures, guidelines, changes to the decision-making process, and changes to the tools or environment used in Agorex development. | ||
|
|
||
|
|
There was a problem hiding this comment.
We should have a feature or exchange proposal outlined here as well.
Or are we keeping AIPs only for organizational matters?
There was a problem hiding this comment.
I assumed AIPs would be used for both the organisation and exchange. How would we go about proposing modifications and new features?
There was a problem hiding this comment.
I would think this AIP process should be allowed for both, but with the exchange, speed of development is also important, so maybe a less formal process. (approved review of 1 board member for merges into master?)
There was a problem hiding this comment.
Ah, yes... This is what I was referring to above. As I wrote here I think we should consider optimizing the document for process/organizational proposals only. I think proposals for new "features" on the exchange should be done via pull requests and issues. For minor features, just do it (or at least start it) and submit a PR. For more significant changes, start an issue to discuss it and get community feedback first.
I think the challenge for a community, open source exchange at first is actually getting enough actual contributors to develop the features, and ideas and agreement on features is much easier. Once we have so many contributors willing to develop features and the more evident problem is just deciding which ones to do, we could formalize the process by updating this doc (with a new AIP, or a PR updating this one).
Ethereum is a bit different in that Ethereum is a protocol and enhancements and changes require of clients (similar to python). I think for the most part the exchange won't have that as such a significant concern.
There was a problem hiding this comment.
I'm on board with that reasoning.
AIPs are primarily focused on organizational matters, but _could _ be used to request large feature changes to the exchange.
proposals/aip-1.md
Outdated
|
|
||
| While the [Agorex Board of Directors](https://github.com/orgs/Agorex-io/teams/board-of-directors) should quite liberally accept Draft status proposals subject to the criteria above, merging a proposal with a **Final** status such as **Final** or **Replaced** requires more review and rigor since, by merging a Final proposal The Board is agreeing to execute the proposed process on behalf of the community. | ||
|
|
||
| Once the proposal is reviewed and approved by The Board on behalf of the community, the status will be changed to "Final" and it will be merged. |
There was a problem hiding this comment.
What happens next? Who is in charge of making sure that proposal makes it into the organization?
Maybe something like:
"At this time, it is the direct responsibility of the board of directors to make sure that this proposal makes it into production. The champion and other community members are encouraged to help this process by providing the proper code or documentation necessary."
There was a problem hiding this comment.
Good point. How about adding the following to optimize for processes instead of features:
By merging the proposal the Board is explicitly accepting the responsibility of executing the proposed process on behalf of the community.
proposals/aip-1.md
Outdated
| Draft -> Withdrawn | ||
| Draft -> Rejected | ||
| Draft -> Deferred | ||
| Final -> Replaced |
There was a problem hiding this comment.
Shouldn't we close any AIP PRs that end up in the middle 3 statuses so as to keep the repository clean?
There was a problem hiding this comment.
EIPs (and the python enhancement proposal that it is derived from) leave them so that people can see the past decision that was rejected for future reference. I think that makes sense.
There was a problem hiding this comment.
Okay, so we will need labels for these different states so that we can filter them out.
There was a problem hiding this comment.
Yes, the status is in the preamble.
proposals/aip-1.md
Outdated
|
|
||
|
|
||
| # Assumptions # | ||
| - Assumes 2FA is required by the GitHub org and all directors on the Board of Directors are using 2FA when submitting PRs, commits, and reviewing proposals. |
proposals/aip-1.md
Outdated
| - ONLY the [@Agorex-io/board-of-directors](https://github.com/orgs/Agorex-io/teams/board-of-directors) can approve proposals that move beyond a PR and end up merged to the repository. | ||
| - [@Agorex-io/proposal-editors](https://github.com/orgs/Agorex-io/teams/proposal-editors) cannot approve, but can facilitate the process and guide champions and authors. They can also edit PRs in progress to help get the preamble and formatting correct according to the workflow. | ||
|
|
||
| - Assumes [required reviews have been set up **to include repository admins**](https://help.github.com/articles/enabling-required-reviews-for-pull-requests) option is on. |
There was a problem hiding this comment.
I set this to "On" and 100% believe it should be always that way.
proposals/aip-1.md
Outdated
|
|
||
|
|
||
| # Open Issues / TODO # | ||
| - [ ] Near the mention of "We use GitHub's [required reviews for pull requests]" above, but that is for every review/pull of the PR. PRs can be merged in a Draft status without being accepted. More final status (Accepted, Deferred, Rejected, and Replaced, Final) require more careful reviews. That needs clarified. Or we could make a different directory for Draft proposals and Accepted/Finalized/Active/Replaced proposals to keep the review/approval requirement lower for Draft proposals. |
There was a problem hiding this comment.
Using different branches for this as long as it is well documented is one option.
I may be misunderstanding your question though.
When should members submit a review that "approve"s a PR? Only when we feel it should be Accepted?
There was a problem hiding this comment.
This was more of a note to myself, and I think is now resolved with the "Acceptance Process" section. I will remove this and we can discuss the Acceptance Process section now.
proposals/aip-1.md
Outdated
| - [ ] Set up the [@Agorex-io/proposal-editors](https://github.com/orgs/Agorex-io/teams/proposal-editors) team. | ||
| - [ ] Set up the [@Agorex-io/board-of-directors](https://github.com/orgs/Agorex-io/teams/board-of-directors) team. | ||
| - Initially proposal-editors and board-of-directors are likely to be the same members. Long term this may scale better to keep them separate. | ||
| - [ ] README.md |
There was a problem hiding this comment.
What should the base readme include?
A summarized version of AIP1 and what this repo is?
There was a problem hiding this comment.
Yes that is my intent. Just housekeeping.
proposals/aip-1.md
Outdated
|
|
||
|
|
||
| # Future Consideration # | ||
| - Use PGP-signed commits? |
There was a problem hiding this comment.
Kind of an unnecessary pain.
We could just require 2 forms of approval: PR review approval + Verified Discord / Email / etc. approval
There was a problem hiding this comment.
I tend to agree, but this was suggestion from the community to consider so I thought I'd would include it here to allow the community to consider it further. We crypto people tend to lean towards the tinfoil hat side of things after all. 😄
There was a problem hiding this comment.
Yeah, good inclusion.
I think we require 2FA on all GH accounts, an approved review, then one confirmation from outside source (Discord, email, etc)
That should cover our bases.
proposals/aip-1.md
Outdated
| ## Proposal Statuses ## | ||
| The possible paths of the status of AIPs are as follows: | ||
|
|
||
| Draft -> Accepted -> Final |
There was a problem hiding this comment.
These all need 2 spaces at the end of the line or they wrap onto the same line.
There was a problem hiding this comment.
Good one. Fixed.
proposals/aip-1.md
Outdated
|
|
||
| - Abstract - a short (~200 word) description of the technical issue being addressed. | ||
|
|
||
| - Copyright/public domain - Each AIP must either be explicitly labeled as placed in the public domain (see this AIP as an example). |
There was a problem hiding this comment.
"either labeled as placed in public domain" or what?
proposals/aip-1.md
Outdated
|
|
||
| - Copyright/public domain - Each AIP must either be explicitly labeled as placed in the public domain (see this AIP as an example). | ||
|
|
||
| - Motivation - The motivation should motivated the proposal including a clear explanation of the problem that the AIP solves and why a current process or (lack thereof) is inadequate to address the problem. AIP submissions without sufficient motivation may be rejected outright. |
There was a problem hiding this comment.
Should be:
"The motivation should explain what motivated..."
proposals/aip-1.md
Outdated
|
|
||
| - Rationale - The rationale fleshes out the proposal by describing why particular decisions were made. It should describe alternatives that were considered and reference any related works. The rationale should provide evidence of consensus within the community and discuss important objections or concerns raised during discussion. | ||
|
|
||
| - Copyright Waiver - All AIPs must be in public domain. See the bottom of this AIP for an example copyright waiver. |
There was a problem hiding this comment.
If this is here, the "copyright/public domain" line above can be removed.
proposals/aip-1.md
Outdated
|
|
||
| - Read the AIP to check if it is ready: sound and complete. The ideas must technically make sense, even if they don't seem likely to be accepted. | ||
| - The title should accurately describe the content. | ||
| - Edit the AIP for language (spelling, grammar, sentence structure, etc.), markup (Github flavored Markdown). |
There was a problem hiding this comment.
Should this be the duty of the editor or the submitter?
There was a problem hiding this comment.
Editors reviews it. They If the AIP isn't ready, an editor will send it back to the author for revision... Generally speaking the author will actually make the changes based on feedback from the editor.
There was a problem hiding this comment.
So then it shouldn't include "Edit the AIP" as a duty of the editor?
Just reviewing the AIP and making suggestions.
There was a problem hiding this comment.
Yep, thanks. Changing this to Work with the author/champion to edit the AIP for language... I don't think there is a problem if the editor graciously does some cosmetic improvements such as spelling and other housekeeping items to the doc itself. I think it is also fine for the editor to ask the author/champion to make required changes. So my intention is to leave it a little open for interpretation here.
proposals/aip-1.md
Outdated
| - Assumes 2FA is required by the GitHub org and all directors on the Board of Directors are using 2FA when submitting PRs, commits, and reviewing proposals. | ||
| - Roles: | ||
| - ONLY the [@Agorex-io/board-of-directors] can approve proposals that move beyond a PR and end up merged to the repository. | ||
| - [@Agorex-io/proposal-editors] cannot approve, but can facilitate the process and guide champions and authors. They can also edit PRs in progress to help get the preamble and formatting correct according to the workflow. |
There was a problem hiding this comment.
In the proposal editor section, it states that the editor will "accept the corresponding pull request". This section seems to state the obvious? Should they mark it as complete, notify a board member, and the board member can pull?
There was a problem hiding this comment.
I reworded the proposal editor section slightly to be more explicit and removed this Assumptions section. The Assumptions section was really more my notes and comments for you, which were later addressed in the core doc so they became repetitive. I don't think appropriate to leave in the doc permanently anyway.
proposals/aip-1.md
Outdated
| - Use PGP-signed commits? | ||
|
|
||
|
|
||
| # References # |
There was a problem hiding this comment.
This markdown simply sets those links.
You'll also need to list them like so:
[the Issues section of this repository]
[the Agorex subreddit]
[the Agorex Discord chat rooms]
[pull request in this repository]
There was a problem hiding this comment.
@JonathonDunford Right, they are all referenced in the doc above. Do you think it will be best to duplicate each reference as an explicit link so that they are visible in the markdown? Or maybe put a comment to explain why they're only visible in the source markdown itself? Or maybe just remove the "References" heading?
There was a problem hiding this comment.
Maybe just remove the references heading.
Everything should be viewed from Markdown IMO
|
I also think that suggestions (like additions, edits, etc.) that get merged into the AIP (even this one) should reference those users in some way. Should there be a separate section for that or should it be in the authors section at the top? |
…on than it is helping.
… Doesn't seem like that should be memorialized in the final doc.
I added an explicit Acknowledgements section. |
|
I have now read through all the changes in the pull request and it all looks good to me. Very good work by you :) You have my stamp of approval 👍 |
|
After that references fix, I think this is good to go and will give my stamp of approval then 👍 |
…Future Consideration as it looked odd there at the bottom.
|
Thanks for the feedback everyone. Please review again and let me know what else you see. |
|
@rowmagnon Will be great if you can review and approve for merging as a draft. Great if you can merge it if you approve! |
Submitting this for discussion (via comments on specific lines) in case someone has burning concerns or other feedback. It is still an incomplete draft, so do not feel compelled to discuss/comment yet.
Do not merge this as a Draft until the following are checked off:
I am requesting a merge of the draft. Please review, edit, and offer feedback so that we can keep it moving. Thanks!