Skip to content

Commit 2951134

Browse files
committed
Address PR feedback
1 parent 18ec282 commit 2951134

File tree

1 file changed

+17
-10
lines changed

1 file changed

+17
-10
lines changed

CONTRIBUTING.md

+17-10
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,10 @@ Contents:
1515
Packages
1616
========
1717

18-
TODO: Put `beacon` in `dom`
18+
TODO: Pending discussion in #545
19+
1920
TODO: Put DOMParser in `dom` with its extra types in its companion object
20-
TODO: Put `gamepad` in `dom`
21-
TODO: Put `push` in `dom`? Everything is `Push*`. Or will there be `Push*` from other APIs too? Do we care?
22-
TODO: Put `sharedworkers` in `dom`
23-
TODO: Put `storage` in `dom`? Does it really warrant its own package?
2421
TODO: Put `webgl/extensions` in `webgl`
25-
TODO: The `ext` package is named confusingly imo. Ideally we could rename it to something clearer.
2622

2723
```
2824
org.scalajs.dom — All top-level facades
@@ -60,6 +56,8 @@ Facades
6056
=======
6157

6258
If a feature has many types that don't share a common unambiguous prefix (like `crypto`),
59+
* firstly, please raise an issue to discuss so that you don't do work only to be asked to undo it
60+
during PR review
6361
* create a feature package
6462
* put all of its types in its package
6563

@@ -81,8 +79,9 @@ Non-Facades
8179
* Add Scala-only utilities that pertain to a specific facade, in the facades companion object
8280
Eg: helper constructors, legal facade values.
8381

84-
* Add Scala-only utilities that don't pertain to a specific facade, or shouldn't be universally
85-
available (subjective judgement here) in the `ext` package.
82+
* We currently don't see the need for Scala-only utilities that don't pertain to a specific facade,
83+
or shouldn't be universally available (subjective judgement here).
84+
If you believe you've got a compelling use case please raise an issue first to discuss.
8685

8786

8887
Partially-Supported DOM API
@@ -94,7 +93,7 @@ TODO: Pending discussion in #514
9493
Binary Compatibility
9594
====================
9695

97-
Binary compatibility for Scala.JS facades is different than standard Scala.
96+
Binary compatibility for Scala.js facades is different than standard Scala.
9897
The following are changes that are indeed incompatible in both formats:
9998

10099
Don't:
@@ -117,8 +116,16 @@ Submitting a PR
117116
Once you're done making your changes...
118117

119118
1. Run `sbt prePR`
119+
120120
2. Run `git diff api-reports` and ensure that you aren't breaking backwards-binary-compatibility
121-
(see above). We'll probably automate this step in the future (See #503)
121+
(see above). The api reports are auto-generated by `prePR` and provide a concise summary of the
122+
entire API. Make sure to look at the top of the file for a description of format.
123+
124+
Note: We might automate binary-compatibility checking in the future (See #503) but for now,
125+
it's just a helpful tool for reviewing PRs.
126+
122127
3. Check in and commit the changes to `api-reports`
128+
123129
4. Submit your PR
130+
124131
5. Know that your contribution is appreciated, thank you!

0 commit comments

Comments
 (0)