Skip to content

Commit b9ea6e8

Browse files
fitzgenfitzgen
authored
Merge pull request #65 from fitzgen/workflow
Expand and formalize contribution workflow a bit
2 parents 20ea7ea + 8eb9b18 commit b9ea6e8

File tree

3 files changed

+214
-31
lines changed

3 files changed

+214
-31
lines changed

‎CONTRIBUTING.md

+116-31
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,12 @@ yourself.
1919
- [Wasm Headless Browser Tests](#wasm-headless-browser-tests)
2020
- [Non-Wasm Tests](#non-wasm-tests)
2121
- [Formatting](#formatting)
22-
- [Gloo Crate Checklist](#gloo-crate-checklist)
23-
- [Designing APIs](#designing-apis)
22+
- [Workflow](#workflow)
23+
- [Proposing a Design](#proposing-a-design)
24+
- [Design Checklist](#design-checklist)
25+
- [Implementation and Feedback Cycle](#implementation-and-feedback-cycle)
26+
- [Implementation Checklist](#implementation-checklist)
27+
- [Team Members](#team-members)
2428

2529
<!-- END doctoc generated TOC please keep comment here to allow auto update -->
2630

@@ -92,34 +96,65 @@ To (re)format the Gloo source code, run:
9296
$ cargo fmt --all
9397
```
9498

95-
## Gloo Crate Checklist
99+
## Workflow
96100

97-
Here is a checklist that all Gloo utility crates should fulfill:
101+
Designing APIs for Gloo, its utility crates, and interfaces between them takes a
102+
lot of care. The design space is large, and there is a lot of prior art to
103+
consider. When coming to consensus on a design, we use a simplified, informal
104+
version of [our RFC process][rfcs], where we have design discussions inside the
105+
Gloo issues tracker.
98106

99-
* [ ] The crate should be named `gloo-foobar`, located at `crates/foobar`, and
100-
re-exported from the umbrella crate like:
107+
> Note: when fixing a bug in a semver-compatible way that doesn't add any new
108+
> API surface (i.e. changes are purely internal) we can skip the design proposal
109+
> part of this workflow, and jump straight to a pull request.
101110
102-
```rust
103-
pub use gloo_foobar as foobar;
104-
```
111+
The graph below gives an overview of the workflow for proposing, designing,
112+
implementing, and merging new crates and APIs into Gloo. Notably, we expect a
113+
large amount of design discussion to happen up front in the issue thread for the
114+
design proposal.
105115

106-
* [ ] The `authors` entry in `Cargo.toml` is "The Rust and WebAssembly Working
107-
Group".
116+
[![Graph showing the workflow of proposing, designing, and merging new crates and
117+
APIs into Gloo](./new-design-workflow.png)](./new-design-workflow.png)
108118

109-
* [ ] Crate's public interface follows the [Rust API Guidelines][api-guidelines].
119+
[rfcs]: https://github.com/rustwasm/rfcs
120+
121+
### Proposing a Design
110122

111-
* [ ] Uses [`unwrap_throw` and `expect_throw`][unwrap-throw] instead of normal `unwrap` and
112-
`expect`.
123+
Before writing pull requests, we should have a clear idea of what is required
124+
for implementation. This means there should be a skeleton of the API in the form
125+
of types and function/method signatures. We should have a clear idea of the
126+
layers of APIs we are exposing, and how they are built upon one another.
127+
128+
Note that exploratory implementations outside of Gloo are encouraged during this
129+
period to get a sense for the API's usage, but you shouldn't send a pull request
130+
until the design has been accepted.
131+
132+
Before the design is accepted, at least two team members must have stated that
133+
they are in favor of accepting the design in the issue thread.
134+
135+
[Here is an issue template you can use for proposing
136+
designs.](https://github.com/rustwasm/gloo/issues/new?assignees=&labels=&template=propose_design.md&title=)
137+
138+
#### Design Checklist
139+
140+
Here is a checklist of some general design principles that Gloo crates and APIs
141+
should follow:
142+
143+
* [ ] Crate's public interface follows the [Rust API Guidelines][api-guidelines].
113144

114145
* [ ] Callback-taking APIs are generic over `F: Fn(A) -> B` (or `FnMut` or
115146
`FnOnce`) instead of taking `wasm_bindgen::Closure`s or
116147
`js_sys::Function`s directly.
117148

118-
* [ ] If the API can be implemented as a Future / Stream, then it should first be implemented as a callback, with the callback API put into the `callback` submodule.
149+
* [ ] If the API can be implemented as a Future / Stream, then it should first
150+
be implemented as a callback, with the callback API put into the
151+
`callback` submodule.
119152

120-
Then the Future / Stream should be implemented using the callback API, and should be put into the `future` or `stream` submodule.
153+
Then the Future / Stream should be implemented using the callback API, and
154+
should be put into the `future` or `stream` submodule.
121155

122-
Make sure that the callback and Future / Stream APIs properly support cancellation (if it is possible to do so).
156+
Make sure that the callback and Future / Stream APIs properly support
157+
cancellation (if it is possible to do so).
123158

124159
* [ ] Uses nice Rust-y types and interfaces instead of passing around untyped
125160
`JsValue`s.
@@ -129,32 +164,82 @@ Here is a checklist that all Gloo utility crates should fulfill:
129164
escape hatch for dropping down to raw `web_sys` bindings when an API isn't
130165
fully supported by the crate yet.
131166

132-
* [ ] There is a loose hierarchy with "mid-level" APIs (which are essentially thin wrappers over the low-level APIs), and "high-level" APIs (which make more substantial changes).
167+
Similar for `from_raw` constructors and `into_raw` conversion methods when
168+
applicable.
169+
170+
* [ ] There is a loose hierarchy with "mid-level" APIs (which are essentially
171+
thin wrappers over the low-level APIs), and "high-level" APIs (which make
172+
more substantial changes).
173+
174+
As a general rule, the high-level APIs should be built on top of the
175+
mid-level APIs, which in turn should be built on top of the low-level APIs
176+
(e.g. `web_sys`)
133177

134-
As a general rule, the high-level APIs should be built on top of the mid-level APIs, which in turn should be built on top of the low-level APIs (e.g. `web_sys`)
135-
136-
There are exceptions to this, but they have to be carefully decided on a case-by-case basis.
178+
There are exceptions to this, but they have to be carefully decided on a
179+
case-by-case basis.
180+
181+
### Implementation and Feedback Cycle
182+
183+
Once we've accepted a design, we can move forward with implementation and
184+
creating pull requests.
185+
186+
The implementation should generally be unsurprising, since we should have
187+
already worked out most of the kinks during the earlier design discussions. If
188+
there are significant new issues or concerns uncovered during implementation,
189+
then these should be brought up in the design proposal discussion thread again,
190+
and the evolved design reaffirmed with two team members signing off once
191+
again.
192+
193+
If there are no new concerns uncovered, then the implementation just needs to be
194+
checked over by at least one team member. They provide code review and feedback
195+
on the implementation, then the feedback is addressed and pull request updated.
196+
Once the pull request is in a good place and CI is passing, a team member may
197+
approve the pull request and merge it into Gloo. If any team member raises
198+
concerns with the implementation, they must be resolved before the pull request
199+
is merged.
200+
201+
#### Implementation Checklist
202+
203+
Here is a checklist that all crate and API implementations in Gloo should
204+
fulfill:
205+
206+
* [ ] The crate should be named `gloo-foobar`, located at `gloo/crates/foobar`,
207+
and re-exported from the umbrella Gloo crate like:
208+
209+
```rust
210+
// gloo/src/lib.rs
211+
212+
pub use gloo_foobar as foobar;
213+
```
214+
215+
* [ ] The `authors` entry in `gloo/crates/foobar/Cargo.toml` is "The Rust and
216+
WebAssembly Working Group".
217+
218+
* [ ] Uses [`unwrap_throw` and `expect_throw`][unwrap-throw] instead of normal
219+
`unwrap` and `expect`.
137220

138221
* [ ] Headless browser and/or Node.js tests via `wasm-pack test`.
139222

140223
* [ ] Uses `#![deny(missing_docs, missing_debug_implementations)]`.
141224

142225
* [ ] Crate's root module documentation has at least one realistic example.
143226

144-
* [ ] Crate has at least a brief description of how to use it in the Gloo guide.
227+
* [ ] Crate has at least a brief description of how to use it in the Gloo guide
228+
(the `mdbook` located at `gloo/guide`).
145229

146230
[unwrap-throw]: https://docs.rs/wasm-bindgen/0.2.37/wasm_bindgen/trait.UnwrapThrowExt.html
147231
[api-guidelines]: https://rust-lang-nursery.github.io/api-guidelines/
148232

149-
## Designing APIs
233+
## Team Members
150234

151-
Designing APIs for Gloo, its utility crates, and interfaces between them takes a
152-
lot of care. The design space is large, and there is a lot of prior art to
153-
consider. When coming to consensus on a design, we use a simplified, informal
154-
version of [our RFC process][rfcs], where we have design discussions inside the
155-
Gloo issues tracker.
235+
Team members sign off on design proposals and review pull requests to Gloo.
156236

157-
[Here is an issue template you can use for proposing
158-
designs.](https://github.com/rustwasm/gloo/issues/new?assignees=&labels=&template=propose_design.md&title=)
237+
* `@fitzgen`
238+
* `@Pauan`
239+
* `@rylev`
240+
* `@yoshuawuyts`
241+
* `@David-OConnor`
159242

160-
[rfcs]: https://github.com/rustwasm/rfcs
243+
If you make a handful of significant contributions to Gloo and participate in
244+
design proposals, then maybe you should be a team member! Think you or someone
245+
else would be a good fit? Reach out to us!

‎new-design-workflow.dot

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Re-render the PNG image with:
2+
//
3+
// dot -Tpng new-design-workflow.dot -o new-design-workflow.png
4+
5+
digraph {
6+
nodesep = 1.5;
7+
8+
// Nodes
9+
10+
start [shape = "point", style = "invis", width = 0];
11+
proposed [label = <<b>Design Proposed</b>>, shape = "circle", style = "filled", fillcolor = "#c5def5"];
12+
accepted [label = <<b>Design Accepted</b>>, shape = "circle", style = "filled", fillcolor = "#c5def5"];
13+
closed [label = <<b>Closed</b>>, shape = "doublecircle", style = "filled", fillcolor = "#f48b9b"];
14+
15+
wip_impl [label = <<b>WIP Implementation</b>>, shape = "circle", style = "filled", fillcolor = "#c5def5"];
16+
changes_needed [label = <<b>Changes Needed</b>>, shape = "circle", style = "filled", fillcolor = "#c5def5"];
17+
merged [label = <<b>Merged into Gloo</b>>, shape = "doublecircle", style = "filled", fillcolor = "#0be27e"];
18+
19+
// Edges
20+
21+
start -> proposed [label = <
22+
<table border="0">
23+
<tr><td align="left" cellpadding="5"><b>📝 Propose API Design</b></td></tr>
24+
<tr><td align="left">Explain motivation and use cases</td></tr>
25+
<tr><td align="left">Alternatives and rationale</td></tr>
26+
<tr><td align="left">Uses API proposal template</td></tr>
27+
</table>
28+
>];
29+
30+
proposed -> proposed [label = <
31+
<table border="0">
32+
<tr><td align="left" cellpadding="5"><b>💬 Design Discussion</b></td></tr>
33+
<tr><td align="left">Determine public types and function signatures</td></tr>
34+
<tr><td align="left">Are the abstractions zero-cost?</td></tr>
35+
<tr><td align="left">Does it expose the onion layers?</td></tr>
36+
<tr><td align="left">Work towards consensus</td></tr>
37+
</table>
38+
>];
39+
40+
proposed -> closed [label = <
41+
<table border="0">
42+
<tr><td align="left" cellpadding="5"><b>❌ Rejected</b></td></tr>
43+
<tr><td align="left">Fatal flaw discovered</td></tr>
44+
<tr><td align="left">Consensus this doesn't belong in Gloo</td></tr>
45+
</table>
46+
>];
47+
48+
proposed -> closed [label = <
49+
<table border="0">
50+
<tr><td align="left" cellpadding="5"><b>📆 Delayed</b></td></tr>
51+
<tr><td align="left">Promising, but don't want to focus on it now</td></tr>
52+
<tr><td align="left">Blocked on something else</td></tr>
53+
</table>
54+
>];
55+
56+
proposed -> accepted [label = <
57+
<table border="0">
58+
<tr><td align="left" cellpadding="5"><b>✔ Accepted</b></td></tr>
59+
<tr><td align="left">Consensus we want this design</td></tr>
60+
<tr><td align="left">Clear how we will implement it</td></tr>
61+
<tr><td align="left">2 or more team members sign off</td></tr>
62+
<tr><td align="left">0 team members register concerns</td></tr>
63+
</table>
64+
>];
65+
66+
accepted -> wip_impl [label = <
67+
<table border="0">
68+
<tr><td align="left" cellpadding="5"><b>🔨 Pull Request Created</b></td></tr>
69+
<tr><td align="left">Adds new <font face="monospace">gloo_whatever</font> crate</td></tr>
70+
<tr><td align="left">Adds tests/docs/examples</td></tr>
71+
</table>
72+
>];
73+
74+
wip_impl -> changes_needed [label = <
75+
<table border="0">
76+
<tr><td align="left" cellpadding="5"><b>🤔 Changes Requested</b></td></tr>
77+
<tr><td align="left">Missing docs/examples/tests</td></tr>
78+
<tr><td align="left">CI is failing</td></tr>
79+
</table>
80+
>];
81+
82+
changes_needed -> wip_impl [label = <
83+
<table border="0">
84+
<tr><td align="left" cellpadding="5"><b>😂 Feedback Addressed</b></td></tr>
85+
<tr><td align="left">Tests/docs/examples fixed</td></tr>
86+
<tr><td align="left">CI is green</td></tr>
87+
</table>
88+
>];
89+
90+
wip_impl -> merged [label = <
91+
<table border="0">
92+
<tr><td align="left" cellpadding="5"><b>🎉 Merge</b></td></tr>
93+
<tr><td align="left">All crate checklist items are checked</td></tr>
94+
<tr><td align="left">CI is green</td></tr>
95+
<tr><td align="left">At least 1 team member approves and merges PR</td></tr>
96+
</table>
97+
>];
98+
}

‎new-design-workflow.png

199 KB
Loading

0 commit comments

Comments
 (0)