Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: embedd asciinema casts #1154

Merged
merged 24 commits into from
Mar 10, 2023
Merged

docs: embedd asciinema casts #1154

merged 24 commits into from
Mar 10, 2023

Conversation

datosh
Copy link
Contributor

@datosh datosh commented Feb 8, 2023

Signed-off-by: Fabian Kammel fk@edgeless.systems

Proposed change(s)

  • Move Dockerfile and scripts to generate casts from https://github.com/edgelesssys/screenrecordings to monorepo so we can generate everything from start to finish
  • Use asciinema-player to directly embedd asciinema casts into our docs (without rendering them as mp4).
    • Use termtosvg to convert a .cast file to svg to display in GitHub README.md, since it does not support JavaScript.
  • Updated workflows for SBOM & CLI verification
  • Added new workflows for create config, delete IAM, create cluster, destroy cluster.
  • Documented process and how to style elements of screen casts.

Open Questions

  • With this solution, are mp4 renderings of the screencast still necessary? For example to upload them to YouTube?
    • No, we will record sessions with OBS if we intend to record webinar style sessions.
  • Should the user run generate-screencasts.sh or should the pipeline do this? I lean to the former, since the output needs to be inspected and docs preview is currently broken.
    • It is fine to make the user run this.

Additional info

  • Please try to generate screencasts from your machine and let me know how it goes. Everything is in containers / shell scripts, but it would be great to confirm it works on another system as well!
  • Currently we still have the spinner in our screencasts. Since the screencasts simulate what a user would do (download an actual released Constellation CLI) we need to re-run screencast generation once v2.6.0 is released.

Checklist

  • Update docs
  • Add labels (e.g., for changelog category)
  • Link to Milestone

@netlify
Copy link

netlify bot commented Feb 8, 2023

Deploy Preview for constellation-docs ready!

Name Link
🔨 Latest commit e9225be
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/640bb855133f970008b0b232
😎 Deploy Preview https://deploy-preview-1154--constellation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@datosh datosh added documentation Improvements or additions to documentation hold This cannot be merged right now labels Feb 8, 2023
@datosh datosh added this to the v2.6.0 milestone Feb 8, 2023
@datosh datosh requested review from m1ghtym0 and flxflx and removed request for flxflx February 8, 2023 12:44
Copy link
Member

@m1ghtym0 m1ghtym0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, this is exactly what I had in mind initially when scripting the asciinema recordings:-)
Makes it easy to manage any additions/changes and keep everything consistent with our docs.
For YT we can do longer "webinars" that we record with OBS.

Copy link
Member

@thomasten thomasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it this way, too!

docs/docs/workflows/sbom.md Outdated Show resolved Hide resolved
docs/screencasts/generate-screencasts.sh Show resolved Hide resolved
@datosh
Copy link
Contributor Author

datosh commented Feb 13, 2023

For those who just want to have a quick peek, it currently looks like this:

image

Copy link
Member

@thomasten thomasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good to me, but I'm not very experienced with some of the technologies used in this PR ;)
There could be a bit more space between the widget and the text below. It looks good when inserting a <br/>, but I'm quite sure this isn't the correct way to do it :D

docs/screencasts/README.md Outdated Show resolved Hide resolved
@datosh
Copy link
Contributor Author

datosh commented Feb 13, 2023

Mostly looks good to me, but I'm not very experienced with some of the technologies used in this PR ;)

Same here 😆 so getting a 👍 already helps. Thanks! I checked with @flxflx , and the goal should be to get this to "good enough" until someone with more CSS/design experience can do a second pass! There are probably ways to get this done cleaner, but I am mostly confident that this should be stable and flexible enough for what we use it for.

I added a horizontal line below the screencast and an info box above. I also tried to match the font size more closely to what the docs already have.

I will keep the technical part as it is and work on scripting the other workflows in our docs.

@datosh
Copy link
Contributor Author

datosh commented Feb 15, 2023

Automated creation of screen recordings for documentation is implemented. I have created / updated the recordings. I think additional recordings should be tackled in their own PR so we can get this one to the finish line.

When working on the recordings I noticed the following points, I will clarify with the CC'd folks:

  • constellation iam create gcp --generate-config ... makes the user manually insert project, region & zone into config, although they are already provided to iam create. Could we fill these values automatically? @msanft
  • Terminate docs regarding terraform.tfstate are outdated. What is the correct update? @msanft
  • The spinner breaks the automatic skipping feature of asciinema. Is there an easy way to disable spinner? debug will disable the spinner but enable too verbose output. @daniel-weisse
    • Piping stderr to a file in expect/asciinema seems to be too hacky.
    • Automated asciinema setup requires tty

@msanft
Copy link
Contributor

msanft commented Feb 15, 2023

Could we fill these values automatically?

Missing GCP region values are / were a bug and have been addressed in #1149.

Terminate docs regarding terraform.tfstate are outdated.

This looks good to me at first glance. What is the exact issue? Or are you not talking about IAM termination / deletion?

@datosh
Copy link
Contributor Author

datosh commented Feb 15, 2023

Could we fill these values automatically?

Missing GCP region values are / were a bug and have been addressed in #1149.

Awesome! I will rebase to include the changes 🙂

Terminate docs regarding terraform.tfstate are outdated.

This looks good to me at first glance. What is the exact issue? Or are you not talking about IAM termination / deletion?

I should have been more specific. I was thrown off by: "You can terminate your cluster using the CLI. For this, you need the Terraform state file named terraform.tfstate in the current directory." in terminate docs.

@msanft msanft mentioned this pull request Feb 17, 2023
2 tasks
@msanft
Copy link
Contributor

msanft commented Feb 17, 2023

I should have been more specific. I was thrown off by: "You can terminate your cluster using the CLI. For this, you need the Terraform state file named terraform.tfstate in the current directory." in terminate docs.

I addressed that in #1212 - let me know if this is the change you expected. I wasn't entirely sure as I didn't write that part of the docs nor was involved with the termination command.

@datosh datosh requested a review from m1ghtym0 February 21, 2023 15:53
@datosh datosh removed the hold This cannot be merged right now label Feb 21, 2023
@datosh
Copy link
Contributor Author

datosh commented Feb 21, 2023

Ready for final review: please see updated PR description.

Copy link
Member

@m1ghtym0 m1ghtym0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified locally on my machine, and everything worked fine!
This is awesome! Once, we have 2.6 without the spinner version, I'll rerun the current set of casts

@derpsteb derpsteb modified the milestones: v2.6.0, v2.7.0 Mar 6, 2023
Fabian Kammel added 5 commits March 10, 2023 15:00
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Fabian Kammel and others added 8 commits March 10, 2023 15:01
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
…cript.

Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
docs/docs/workflows/sbom.md Outdated Show resolved Hide resolved
docs/screencasts/docker/check-sbom.expect Outdated Show resolved Hide resolved
docs/screencasts/docker/check-sbom.expect Show resolved Hide resolved
docs/screencasts/docker/configure-cluster.expect Outdated Show resolved Hide resolved
docs/screencasts/docker/create-cluster.expect Show resolved Hide resolved
docs/screencasts/docker/create-cluster.expect Show resolved Hide resolved
docs/screencasts/docker/delete-iam.expect Outdated Show resolved Hide resolved
docs/screencasts/docker/verify-cli.expect Outdated Show resolved Hide resolved
docs/screencasts/docker/verify-cli.expect Show resolved Hide resolved
m1ghtym0 and others added 2 commits March 10, 2023 19:29
Co-authored-by: 3u13r <lc@edgeless.systems>
@m1ghtym0 m1ghtym0 requested a review from thomasten March 10, 2023 18:30
Copy link
Member

@thomasten thomasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Pls publish docs to 2.6 and then it should be ready to merge.

@@ -0,0 +1,138 @@
{"version": 2, "width": 0, "height": 0, "timestamp": 1678455337, "env": {"SHELL": "/bin/bash", "TERM": "xterm-256color"}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete file

@m1ghtym0 m1ghtym0 merged commit 566924c into main Mar 10, 2023
@m1ghtym0 m1ghtym0 deleted the feat/docs/asciinema branch March 10, 2023 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants