chore: platform design proposal process #14065
Conversation
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
…tion of what requires a proposal Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
Signed-off-by: Kelly Greco <kelly@swirldslabs.com>
poulok
requested review from
lpetrovic05,
anthony-swirldslabs,
mxtartaglia-sl and
OlegMazurov
| Requests for immaterial changes can be addressed by pushing additional commits to the existing proposal | ||
| PR while in `Voting`. Examples of immaterial changes are spelling or grammar errors, wording changes that do not modify | ||
| meaning or intent, and formatting changes. Examples of material changes include changes to behaviors or APIs, addition | ||
| or removal of diagrams, etc. If any material changes are needed, the PR must be closed and moved to the `Superseded` | ||
| status. A new proposal PR is prepared and voting is restarted. The `Superseded` PR must reference the new PR to | ||
| create an auditable history. |
There was a problem hiding this comment.
-1
I am strongly opposed to the idea that the PR must be closed after changes.
This defeats the entire purpose of using a PR to do proposals. It creates a ton of extra paperwork, and clears away the comment history. If we want to do proposals like this, it will need to be by decree of management. I don't think I can be convinced vote for this pattern.
There was a problem hiding this comment.
Please see my response on the previous PR where you made this comment.
We are going to try it this way as a starting point.
There was a problem hiding this comment.
Do we need a new branch? Or just a new PR?
There was a problem hiding this comment.
I acknowledge that you have the authority to overrule this vote (and so it's not a true veto). But I don't think I can vote anything but -1 in this circumstance.
There was a problem hiding this comment.
Our process must be optimized for efficiency in building understanding and gaining rough consensus.
On the one hand, having to take a bunch of extra steps to close a PR, open a new one, go to another page, mark one as superseded by the other and put one in draft or voting stage, is not something I'd really want to do. It seems like some of that could be optimized out.
On the other hand, the friction of taking those steps will cause me to put things in draft before it goes to vote, and have to really work on the design and building consensus before it goes to a vote. And that means I am more likely to put the extra effort in and actually make it easier and more productive for people to review and vote. Hopefully this is not the first time they've seen it and they're already primed to vote +1 before I even convert it from draft to ready for review.
If that is true, then maybe the extra friction of closing a failed PR isn't that big of a deal, because it shouldn't happen very often if I only moved to vote after the draft had been widely reviewed.
So I think if we put our energy into good drafts and good conversations on drafts then the voting stage should be mostly a formal affair, and not where most conversation will happen.
Node: HAPI Test (Smart Contract) Results 72 files 72 suites 24m 31s ⏱️ Results for commit f754275. |
Node: HAPI Test (Restart) Results3 tests 3 ✅ 7m 50s ⏱️ Results for commit f754275. |
Node: HAPI Test (Time Consuming) Results19 tests 19 ✅ 22m 41s ⏱️ Results for commit f754275. |
Node: HAPI Test (Crypto) Results 24 files 24 suites 12m 33s ⏱️ Results for commit f754275. |
Node: HAPI Test (Node Death Reconnect) Results3 tests 3 ✅ 5m 41s ⏱️ Results for commit f754275. |
Node: HAPI Test (Token) Results 20 files 20 suites 5m 55s ⏱️ Results for commit f754275. |
Node: HAPI Test (Misc) Results 51 files 51 suites 21m 2s ⏱️ Results for commit f754275. |
Node: Unit Test Results 1 545 files 1 545 suites 2h 58m 58s ⏱️ Results for commit f754275. |
| 1. The proposal has been in the Voting state for at least 3 business days. Business days are defined as SwirldsLabs | ||
| working days and excludes weekends and company holidays. |
There was a problem hiding this comment.
I don't really like defining business days as SwirldsLabs working days. This won't fly in the open source world. I think we should say something more definitive like 72 hrs if posted Mon-Thurs or 96 hrs if posted on Fri-Sun. Or something like that.
There was a problem hiding this comment.
Why can't we publish the company holidays on the website? That way it is transparent and public, and no one feels like they need to come in on an off day to review something.
There was a problem hiding this comment.
If we do nix the Swirlds Labs holidays, I prefer specifying the time in business days defined as M-F. This is more intuitive to people.
There was a problem hiding this comment.
yes, just call it business days.
There was a problem hiding this comment.
As an open source project, SL calendar doesn't have any pre-eminent role. Plus, the dates have on or off are not consistent -- our colleagues in Europe or other nations have completely different holidays.
There was a problem hiding this comment.
I will modify the language to specify business days as M-F, no holidays taken into account.
| ## Voting on A Proposal | ||
|
|
||
| Votes follow the [Apache Voting](https://www.apache.org/foundation/voting.html#expressing-votes-1-0-1-and-fractions) | ||
| scheme. Votes are cast in comments on the PR on a range between -1 to -0, +0 to +1 with the following semantics: |
There was a problem hiding this comment.
How are votes cast? Are they on an inline comment like Cody did above? Or are they in the general comments on the "Conversation" tab?
There was a problem hiding this comment.
I'm flexible. Do you think this needs to be explicitly stated?
There was a problem hiding this comment.
Maybe it should be stated explicitly. Cody's "-1" inline comment could be interpreted as a vote against the entire proposal, or just expressing his opinion about that one item and not a vote against the proposal in general.
There was a problem hiding this comment.
I think it won't hurt to explicitly state this. I just made the mistake of approving with a vote, which turned out to not be the way to do it.
| must be editable (i.e. no images). The only exception is for images that are generated by code, like the wiring diagram. | ||
| These may be drawn on to indicate the intended changes and included as an image. | ||
|
|
||
| If any APIs were developed in code as part of the design process, it may be included in the proposal directly or in the |
There was a problem hiding this comment.
I don't think code in this directory will be picked up as code or included in coverage since it won't compile.
Please do not "approve" the PR unless your vote is a +1 |
| must be editable (i.e. no images). The only exception is for images that are generated by code, like the wiring diagram. | ||
| These may be drawn on to indicate the intended changes and included as an image. | ||
|
|
||
| If any APIs were developed in code as part of the design process, it may be included in the proposal directly or in the |
There was a problem hiding this comment.
I don't think code in this directory will be picked up as code or included in coverage since it won't compile.
| 1. The proposal has been in the Voting state for at least 3 business days. Business days are defined as SwirldsLabs | ||
| working days and excludes weekends and company holidays. |
There was a problem hiding this comment.
yes, just call it business days.
|
|
||
| 1. The proposal has been in the Voting state for at least 3 business days. Business days are defined as SwirldsLabs | ||
| working days and excludes weekends and company holidays. | ||
| 2. The proposal must have at least three +1 votes from platform code owners. |
There was a problem hiding this comment.
I had a discussion with @netopyr today regarding apis that might be used by other teams (services, smart contracts, ...). How do we handle that? I want that an api change in base is reviewed by that parties, too.
There was a problem hiding this comment.
All impacted parties should be coordinated with before the voting phase to ensure that the proposed API makes sense for all stakeholders. This is part of the Architectural Acceptance phase that is outside the scope of this process.
|
+1 While I would not call it 100% complete or perfect, having this initial process defined is a really good start. |
|
-1 In response to #14065 (comment), making it clear that I am voting against the proposal as a whole. Core reason is the requirement to close and reopen the PR after each modification. |
| contain diagrams and supporting materials. Diagrams must be created with DrawIO and exported in SVG format. All content | ||
| must be editable (i.e. no images). The only exception is for images that are generated by code, like the wiring diagram. |
There was a problem hiding this comment.
Why lock the tool used to generate diagrams? For example, there are easier and more automated tools than drawIO for generating UML diagrams. As providing a diagram is in service of the people reading the proposal, it should be up to the owner to choose which tool balances the effort and is more suitable for creating them case by case.
There was a problem hiding this comment.
Also, when using Intellij, it is easy to paste an image, and it automatically adds and links the file to the document in the position you pasted the image. It facilitates adding the image, but the images are generated in PNG format, not SVG
There was a problem hiding this comment.
I also agree that making drawIO a hard requirement is super limiting.
There was a problem hiding this comment.
There are two reasons for naming drawIO as the diagram tool of choice:
- consistency
- it's a free tool available to all, and is therefore open-source friendly
The company made this decision a couple of years ago that all diagrams in the repo would be created with drawIO for these reasons.
There was a problem hiding this comment.
One of my secret desires is to create a new open alternative to draw.io that is both really great to use and has deep integration into github and IntelliJ. Sigh.
The core argument for Draw.io is that anyone can edit it. If we just take a PNG and check it in, then if someone else creates a PR to modify the diagram, they have to recreate it in their tooling to be able to edit it. I think the pain of using draw.io is much less than the pain of having to rebuild a diagram.
The wiring diagram (which is called out as an exception) is different because it can be auto-generated from source, so anyone can edit it.
There was a problem hiding this comment.
I'm already a victim of this policy as I used Lucid to draw a beautiful diagram of shining awesomeness, and now I will have to rebuild it in the dull colors and fonts of Draw.io. Boo.
But I agree with the policy despite that, because no-one else (outside SL) has a hope of contributing to that diagram. And that's wrong. So I should stop using Lucid. Sigh.
There was a problem hiding this comment.
Lucid can export diagrams in drawIO format =)
There was a problem hiding this comment.
I understand the reasoning. Still missing the possibility of using:
Or even the diagrams generated by IntelliJ, seems like a loss.
|
+0.6. |
|
Superseded by #14083 |
Description:
Introduces documentation for the new Platform Design Proposal process.
Related issue(s):
Fixes #13793
Draft version: #13870