Setup docs app #44

simonihmig · 2023-02-13T17:00:58Z

This is only the boilerplate of the docs app, copied from ember-toucan-core, and enabling CI/CD. Writing the actual documentation will follow in separate PR(s).

changeset-bot · 2023-02-13T17:01:03Z

⚠️ No Changeset found

Latest commit: c324e7a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

simonihmig · 2023-02-14T12:23:36Z

@NullVoxPopuli this message seems to suggest we need to create a new project for this? Do you have the permissions, I don't have any cloudflare access on behalf of CS, right?

NullVoxPopuli · 2023-02-14T13:51:46Z

@NullVoxPopuli this message seems to suggest we need to create a new project for this?

I just made it 🎉

I don't have any cloudflare access on behalf of CS, right?

Right, we can get you added if you want (UX has its own cloudflare account). But for this kind of setup you also would need to be a GitHub org admin. :(

github-actions · 2023-02-14T14:30:30Z

Preview URLs

Env: preview
Docs: https://6286b89f.ember-headless-form.pages.dev

ynotdraw · 2023-02-16T14:01:52Z

.github/workflows/ci.yml

+          persist-credentials: false
+      - uses: ./.github/actions/pnpm
+      - run: pnpm build
+      # - uses: ./.github/actions/download-built-package


Can this be removed since we have the build run above+below?

The other jobs also still have this. We should clean up all of this of course, when we have the final solution, which I hope #52 will bring!

ynotdraw · 2023-02-16T14:02:39Z

.github/workflows/ci.yml

+          persist-credentials: false
+      - uses: ./.github/actions/pnpm
+      - run: pnpm build
+      # - uses: ./.github/actions/download-built-package


Same question here as above

ynotdraw · 2023-02-16T14:02:56Z

.github/workflows/ci.yml

+            ## Preview URLs
+            Env: ${{ needs.PublishDocstoCloudflarePages.outputs.env }}
+            Docs: ${{ needs.PublishDocstoCloudflarePages.outputs.url }}
+#            # api docs: ${{ needs.PublishDocstoCloudflarePages.outputs.url }}/api/modules.html


I should maybe do this as well in toucan-core, as we don't have api docs yet!

ynotdraw · 2023-02-16T14:06:58Z

docs-app/app/components/feature-card.gts

+  </div>
+</template>;
+
+export default FeatureCard;


Maybe we can/should move these over to oss-docs eventually? (Not part of this PR, obvs, low priority!)

Yeah, I think this was even mentioned somewhere, in a comment or so. Just vaguely remember...

docs-app/app/index.html

docs-app/ember-cli-build.js

ynotdraw · 2023-02-16T14:13:19Z

docs-app/package.json

+    "highlight.js": "^11.6.0",
+    "highlightjs-glimmer": "^1.4.1",
+    "tracked-built-ins": "^3.1.0"
+  }


Do you need the dependenciesMeta here as well? E.g.,:

"dependenciesMeta": { "ember-headless-form": { "injected": true } }

(https://github.com/CrowdStrike/ember-toucan-core/blob/main/docs-app/package.json#L134)

It seems like it was working without it based on what I read from a previous PR (let me dig it up)...but then how come we need it in other packages?? (don't feel the need to answer this if it's going to take a lot of investigation btw, just curious)

Update, yeah you are right it was added over here: https://github.com/CrowdStrike/ember-headless-form/pull/49/files#diff-b03f4acb30e04ceb12a40763ef649759b3f632df2357bcec14fab6dd29ebbfb4R124 in the test app

It wasn't needed as I wasn't really using the addon in the docs app yet. But yes, your are right, we should do that here also - and I just did that, to prevent any more foot guns due to this unfortunate issue.

ynotdraw · 2023-02-16T14:16:57Z

docs-app/tests/helpers/index.js

+  // Additional setup for unit tests can be done here.
+}
+
+export { setupApplicationTest, setupRenderingTest, setupTest };


Totally non-blocking quesiton: We have all of the same in toucan-core under docs-app/tests, but I'm curious if it makes sense for our docs-apps to even have the tests/ directory? We normally use the test-app for testing, so just curious on thoughts around this.

over time docs may have demos complex enough where you want them tested

Ah, that makes sense 👍

Yeah, and I would also like to add even just a very simple smoke test for the docs app, mainly checking that the build does not blow up, and at least something is rendered. This has proven to be useful in other projects like ember-bootstrap, to make sure that e.g. automated renovate updates don't accidentally the docs app!

nicolechung · 2023-02-16T14:39:55Z

docs-app/.prettierrc.js

+'use strict';
+
+module.exports = {
+  plugins: ['prettier-plugin-ember-template-tag'],


!! Should we be using this in our other packages? Does it format everything in between <template> tags (is this what it does?)

We have this already in the test-app for example. It makes prettier even understand the existence of <template>, otherwise it wouldn't be able to parse the file (as it's not valid JS/TS). And yes, it does also format everything inside of the template!

nicolechung · 2023-02-16T14:41:19Z

docs-app/README.md

+
+## Running / Development
+
+- `ember serve`


Question...are we really running ember serve or is it pnpm start? Or should updating this README be in a followup?

they are equiv 🎉

This was copy-paste, and yes, currently there is no difference. However, should we start to add turbo also to scripts in the workspace, including start, then it would start to matter!

Co-authored-by: Tony Ward <8069555+ynotdraw@users.noreply.github.com>

simonihmig · 2023-02-17T10:31:59Z

@NullVoxPopuli I took the freedom to refactor the docs CD workflow a bit, in this commit. Previously we had a separate PostPreviewURLascommenttoPR job, which seemed too much to me tbh. It did a pnpm i and pnpm build, which seems unnecessary for the sticky-pull-request-comment action. And if you remove those, what is left then is a single step that takes 1s, which IMO doesn't justify a separate job. So I merged the two together.

Btw, that sticky-pull-request-comment action wouldn't really be needed, if coudflare would merge and release this bugfix, as the cloudflare action will create a deployment Github event that makes GH render it as a deployment in its native UI (which due to the bug doesn't work correctly for PRs rn). We could still leave the automated comment in here if we think that's useful though.

simonihmig force-pushed the docs-setup branch 2 times, most recently from 56b0ba6 to f935bdd Compare February 13, 2023 17:08

simonihmig marked this pull request as draft February 13, 2023 17:08

simonihmig force-pushed the docs-setup branch from f935bdd to d4372e3 Compare February 14, 2023 14:26

simonihmig mentioned this pull request Feb 15, 2023

Add dependenciesMeta.*.injected, fix peer deps #49

Merged

simonihmig added 4 commits February 16, 2023 09:43

Setup docs-app

ec8908f

Enable CI/CD for docs

cdf53f1

Cleanup docs dependencies

ca1b475

Fix lockfile

d132285

simonihmig force-pushed the docs-setup branch from d5ad508 to d132285 Compare February 16, 2023 08:46

simonihmig marked this pull request as ready for review February 16, 2023 08:56

simonihmig requested review from NullVoxPopuli, ynotdraw and nicolechung February 16, 2023 08:56

ynotdraw reviewed Feb 16, 2023

View reviewed changes

nicolechung reviewed Feb 16, 2023

View reviewed changes

nicolechung approved these changes Feb 16, 2023

View reviewed changes

simonihmig and others added 2 commits February 17, 2023 10:08

Apply suggestions from code review

77c6733

Co-authored-by: Tony Ward <8069555+ynotdraw@users.noreply.github.com>

Use injected also for docs-app

77d0f9c

simonihmig force-pushed the docs-setup branch 2 times, most recently from 7c1bb48 to 03b88ac Compare February 17, 2023 10:18

Post preview URLs in same job

c324e7a

simonihmig force-pushed the docs-setup branch from 03b88ac to c324e7a Compare February 17, 2023 10:22

simonihmig requested a review from ynotdraw February 17, 2023 10:32

simonihmig mentioned this pull request Feb 17, 2023

Setup turborepo, second try #52

Merged

ynotdraw approved these changes Feb 17, 2023

View reviewed changes

simonihmig merged commit ebdd267 into main Feb 20, 2023

simonihmig deleted the docs-setup branch February 20, 2023 09:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup docs app #44

Setup docs app #44

simonihmig commented Feb 13, 2023

changeset-bot bot commented Feb 13, 2023 •

edited

simonihmig commented Feb 14, 2023

NullVoxPopuli commented Feb 14, 2023

github-actions bot commented Feb 14, 2023 •

edited

ynotdraw Feb 16, 2023

simonihmig Feb 17, 2023

ynotdraw Feb 16, 2023

ynotdraw Feb 16, 2023

ynotdraw Feb 16, 2023

simonihmig Feb 17, 2023

ynotdraw Feb 16, 2023

nicolechung Feb 16, 2023

nicolechung Feb 16, 2023

simonihmig Feb 17, 2023

ynotdraw Feb 16, 2023

NullVoxPopuli Feb 16, 2023

ynotdraw Feb 16, 2023

simonihmig Feb 17, 2023

nicolechung Feb 16, 2023

simonihmig Feb 17, 2023

nicolechung Feb 16, 2023

NullVoxPopuli Feb 16, 2023

simonihmig Feb 17, 2023

simonihmig commented Feb 17, 2023

Setup docs app #44

Setup docs app #44

Conversation

simonihmig commented Feb 13, 2023

changeset-bot bot commented Feb 13, 2023 • edited

⚠️ No Changeset found

simonihmig commented Feb 14, 2023

NullVoxPopuli commented Feb 14, 2023

github-actions bot commented Feb 14, 2023 • edited

Preview URLs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonihmig commented Feb 17, 2023

changeset-bot bot commented Feb 13, 2023 •

edited

github-actions bot commented Feb 14, 2023 •

edited