Re: [whatwg/storage] Editorial: Add a diagram for storage model (PR #152)

@domenic commented on this pull request.



> @@ -0,0 +1,10 @@
+erDiagram
+
+%% This is the source file for `model-diagram.svg`.
+%% Build command: `npm i && npm run build-diagram`
+
+"User agent" ||--|| "Storage shed" : "holds (local)"
+"Browsing session" ||--|| "Storage shed" : "holds (session)"

Please change "Browsing session" to "Top-level traversable", per the latest spec.

> @@ -0,0 +1,14 @@
+{
+  "name": "storage",

I suggest moving `"private": true` to the top, and removing `"name"` and `"version"`. Compare to https://github.com/whatwg/webidl/blob/main/package.json.

> @@ -0,0 +1,50 @@
+<svg xmlns="http://www.w3.org/2000/svg" aria-labelledby="chart-title-mermaid-1669067354847 chart-desc-mermaid-1669067354847" style="max-width:396.227px;background-color:#fff;font-family:&quot;trebuchet ms&quot;,verdana,arial,sans-serif;font-size:16px;fill:#333" viewBox="0 0 396.227 815">

Do we want to check in this build output? Or should it be generated on each deploy?

If the latter, we need to do some work to make this repo more like webidl. (Which is another repo which uses Node as part of its build process.) We can make the changes as part of this PR, but we'd also need to update spec-factory so that it generates the files appropriately next time in the same way.

> @@ -0,0 +1,50 @@
+<svg xmlns="http://www.w3.org/2000/svg" aria-labelledby="chart-title-mermaid-1669067354847 chart-desc-mermaid-1669067354847" style="max-width:396.227px;background-color:#fff;font-family:&quot;trebuchet ms&quot;,verdana,arial,sans-serif;font-size:16px;fill:#333" viewBox="0 0 396.227 815">

This file will need to be added as an `"extra_files` in https://github.com/whatwg/spec-factory/blob/main/factory.json otherwise the build will not work. That updates the Makefile like so: https://github.com/whatwg/encoding/blob/main/Makefile#L23 .

We'll need to be sure to *exclude* all the other files. So maybe something like `assets/*.svg`?

Alternately, you could have all the other stuff live in `scripts/`, and have it generate a new file in `assets/` or `images/`. Then you'd add `assets/*.*` as the extra files.

> @@ -0,0 +1,50 @@
+<svg xmlns="http://www.w3.org/2000/svg" aria-labelledby="chart-title-mermaid-1669067354847 chart-desc-mermaid-1669067354847" style="max-width:396.227px;background-color:#fff;font-family:&quot;trebuchet ms&quot;,verdana,arial,sans-serif;font-size:16px;fill:#333" viewBox="0 0 396.227 815">

This image is unreadably small when viewed standalone. https://raw.githubusercontent.com/whatwg/storage/651b26d44e2572123dbc82f93adb61359ed71dda/assets/model-diagram.svg

Will it be extremely small when embedded in the spec, too?

> +      <path fill="none" stroke="gray" d="M3 9v18m6-9q18-18 36 0-18 18-36 0"/>
+    </marker>
+  </defs>
+  <defs>
+    <marker id="c" markerHeight="36" markerWidth="57" orient="auto" refX="39" refY="18">
+      <circle cx="9" cy="18" r="6" fill="#fff" stroke="gray"/>
+      <path fill="none" stroke="gray" d="M21 18q18-18 36 0-18 18-36 0"/>
+    </marker>
+  </defs>
+  <path fill="none" stroke="gray" marker-end="url(#a)" marker-start="url(#b)" d="M73.762 95v8.333c0 8.334 0 25 10.862 41.667 10.863 16.667 32.588 33.333 43.45 41.667L138.937 195" class="er relationshipLine"/>
+  <path fill="none" stroke="gray" marker-end="url(#a)" marker-start="url(#b)" d="M301.875 95v8.333c0 8.334 0 25-10.863 41.667-10.862 16.667-32.587 33.333-43.45 41.667L236.7 195" class="er relationshipLine"/>
+  <path fill="none" stroke="gray" marker-end="url(#c)" marker-start="url(#b)" d="M187.818 270v100" class="er relationshipLine"/>
+  <path fill="none" stroke="gray" marker-end="url(#a)" marker-start="url(#b)" d="M187.818 445v100" class="er relationshipLine"/>
+  <path fill="none" stroke="gray" marker-end="url(#d)" marker-start="url(#b)" d="M187.818 620v100" class="er relationshipLine"/>
+  <path fill="#f0fff0" fill-opacity="100%" stroke="gray" d="M20 20h107.523v75H20z" class="er entityBox"/>
+  <text class="er entityLabel" dominant-baseline="middle" style="font-family:&quot;trebuchet ms&quot;,verdana,arial,sans-serif;font-size:16px" text-anchor="middle" transform="translate(73.762 57.5)">User agent</text>

I wonder why these style attributes exist, despite the overall SVG element already having that style. Oh well; I assume there's nothing we can do.

> @@ -1,3 +1,5 @@
 /storage.spec.whatwg.org/
 /deploy.sh
 /storage.html
+/node_modules/
+package-lock.json

I'm unsure on whether or not we should check in package-lock.json. It looks like we don't for other Node-using spec repos. But it's generally good practice. We check it in for participate.whatwg.org.

If you check it in, you get reproducible builds. If you don't check it in, all your dependencies-of-dependencies will get recomputed each time, potentially with inadvertent breaking changes.

However, if you check it in, you also start getting annoying GitHub security alerts about all your dependencies-of-dependencies :-/.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/storage/pull/152#pullrequestreview-1191123234
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/storage/pull/152/review/1191123234@github.com>