Merge pull request #523 from scala-js/topic/standards

Document coding standards and conventions

Author: armanbilge

CONTRIBUTING.md

@@ -0,0 +1,133 @@
+Guide to Contributing
+=====================
+
+Thanks for contributing to scala-js-dom!
+We primarily accept PRs for:
+* Adding facades for APIs documented in the spec
+* Enhancing/fixing existing facades to match the spec
+* Adding non-facade Scala utilities to complement specific facades
+* Any other bug fixes etc.
+
+If you would like to make PR that doesn't fall under those categories,
+please raise an issue first for discussion so we can give you the go-ahead!
+
+We look forward to your PRs!
+
+Contents:
+
+* Packages
+* Files
+* Facades
+* Non-Facades
+* Binary Compatibility
+* Submitting a PR
+
+
+Packages
+========
+
+In v1.x, there used to be sub-packages grouping some parts of the DOM API by major feature.
+In v2.x, we've decided to put everything in `org.scalajs.dom` and get rid of sub-packages.
+
+The reason for this change is that the real DOM API isn't namespaced in anyway, and the decision
+whether to group in a package or not, was subjective and inconsistent.
+
+
+Files
+=====
+
+* Use `package.scala` for a package object and nothing else.
+  Also don't include traits/classes/objects.
+
+* Match the filename to the trait/class/object in it; don't add multiple top-level types.
+  This is effectively Java-style.
+
+
+Facades
+=======
+
+We accept facades for any non-deprecated API documented in the spec, including experimental APIs or APIs not supported on all browsers.
+* MDN: https://developer.mozilla.org/en-US/docs/Web/API
+* WHATWG: https://spec.whatwg.org/
+
+Please:
+* Use `def` for read-only properties unless there is a compelling reason it should be a `val`
+  (i.e., the spec definitively states it is constant)
+* Use `Double` for integer-values that can fall outside the range of `Int`
+* Add scaladocs via copy-paste from MDN
+
+
+Non-Facades
+===========
+
+* Implicit conversions should go in companion objects so that they are always in scope without the
+  need for imports. There's no need to group by feature, the types already specify the feature.
+
+* Add Scala-only utilities that pertain to a specific facade, in the facades companion object
+  Eg: helper constructors, legal facade values.
+
+* We currently don't see the need for Scala-only utilities that don't pertain to a specific facade,
+  or shouldn't be universally available (subjective judgement here).
+  If you believe you've got a compelling use case please raise an issue first to discuss.
+
+Binary Compatibility
+====================
+
+Binary compatibility for Scala.js facades is different than standard Scala.
+The following are changes that are indeed incompatible in both formats:
+
+Don't:
+  * Remove a trait / class / object
+  * Change a class into a trait or vice versa
+
+Here is a non-exhaustive list of changes that would be binary-incompatible for Scala classes, but
+are compatible for JS facade types:
+
+You can:
+  * Remove a member
+  * Change the type or signature of a member
+  * Add a field in a trait
+  * Add an abstract member in a trait
+
+To help us enforce binary compatibility, we use API reports.
+They are auto-generated by running `prePR` in sbt and provide a concise summary of the entire API.
+Note: We might automate binary compatibility checking in the future (see #503) but for now,
+it's just a helpful tool for reviewing PRs.
+
+Here is an example of a binary _compatible_ change in #491 as it appears in the API report diff.
+Note that `[JC]` stands for **J**avascript **C**lass, indicating that `HTMLAudioElement` is a facade type and thus this is a compatible change.
+```diff
+-raw/HTMLAudioElement[JC] def play(): Unit
++raw/HTMLAudioElement[JC] def play(): js.UndefOr[js.Promise[Unit]]
+```
+
+Here is an example of a binary _incompatible_ change in #458 as it appears in the API report diff.
+Even though the `Fetch` object is a facade (a **J**avascript **O**bject), moving it out of the `experimental` package is not binary compatible.
+(In this particular case, the change was accepted along with many binary-breaking changes going into 2.0.0.)
+```diff
+-experimental/Fetch[JO] def fetch(info: RequestInfo, init: RequestInit = null): js.Promise[Response]
++Fetch[JO] def fetch(info: RequestInfo, init: RequestInit = null): js.Promise[Response]
+```
+
+Anything annotated `[SC]`, `[ST]`, or `[SO]` in an API report is an ordinary Scala class/trait/object,
+for which standard binary compatibility rules apply.
+
+If the above doesn't make sense to you, don't worry!
+The majority of useful changes to scala-js-dom are indeed binary compatible.
+
+
+Submitting a PR
+===============
+
+Once you're done making your changes...
+
+1. Run `sbt prePR`
+
+2. Run `git diff api-reports` and ensure that you aren't breaking backwards-binary-compatibility
+   (see above).
+
+3. Check in and commit the changes to `api-reports`
+
+4. Submit your PR
+
+5. Know that your contribution is appreciated, thank you!