8000 docs: embedd asciinema casts by datosh · Pull Request #1154 · edgelesssys/constellation · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

docs: embedd asciinema casts #1154

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.

Sign up for GitHub

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

Jump to bottom
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 •
edited
Loading

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

@datosh datosh requested a review from thomasten as a code owner February 8, 2023 11:24
@netlify
Copy link
netlify bot commented Feb 8, 2023 •
edited
Loading

✅ 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
m1ghtym0
m1ghtym0 approved these changes Feb 8, 2023
View reviewed changes
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.

thomasten
thomasten reviewed Feb 8, 2023
View reviewed changes
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!

@datosh datosh requested a review from thomasten February 13, 2023 13:09
@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

thomasten
thomasten reviewed Feb 13, 2023
View reviewed changes
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

@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
Merged
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.

m1ghtym0
m1ghtym0 approved these changes Feb 22, 2023
View reviewed changes
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
@m1ghtym0
Embedd asciinema into docs.
e0a4b26 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
e2e screencast generation and docs embedding
5f9cc52 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Colored prompt in asciinema.
ad103e6 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Figure out and document styling of player.
2f5bb77 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Cleanup and work in feedback.
ed2e4e4 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
Fabian Kammel and others added 8 commits March 10, 2023 15:01
@m1ghtym0
Fix styling.
ef815f8 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Improve styling, add explanation for screencasts, update verify cli s…
9e1c91f 
…cript.

Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Screencasts for create, config and terminate.
c86f8a8 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Polish screen casts.
1e74dfd 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Fix shellcheck issues.
2b27c9f 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Re-generate screencasts.
029585b 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
Finalize workflow for README svg.
4edcd5c 
Signed-off-by: Fabian Kammel <fk@edgeless.systems>
@m1ghtym0
update recordings without spinner
ea8a9e9
@m1ghtym0 m1ghtym0 force-pushed the feat/docs/asciinema branch from 30f214b to ea8a9e9 Compare March 10, 2023 14:02
@m1ghtym0 m1ghtym0 requested a review from thomasten March 10, 2023 14:02
thomasten
thomasten requested changes Mar 10, 2023
View reviewed changes
m1ghtym0 and others added 6 commits March 10, 2023 16:05
@m1ghtym0
fix linter
7cccf92
@m1ghtym0 @thomasten
Apply suggestions from code review
1e9509e 
Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>
@m1ghtym0
Shellfmt 2nd try
318895a
@m1ghtym0
Remove pwd from prompt
25a48fb
@m1ghtym0
remove iam-delete cast
< F438 input type="hidden" name="id" value="C_kwDOH5WUPdoAKDY2MjlkMmMxN2ZkMDFiYzlkMzkyMTFhOGFiYjc0YWIzMjBlZWZlYWM" autocomplete="off" data-targets="batch-deferred-content.inputs" />
6629d2c
@thomasten
fix linter
3124e49
3u13r
3u13r reviewed Mar 10, 2023
View reviewed changes
m1ghtym0 and others added 2 commits March 10, 2023 19:29
@m1ghtym0
Rerun recordings
b2dea65
@m1ghtym0 @3u13r
Update docs/screencasts/docker/Dockerfile
53fa3b2 
Co-authored-by: 3u13r <lc@edgeless.systems>
@m1ghtym0 m1ghtym0 requested a review from thomasten March 10, 2023 18:30
thomasten
thomasten approved these changes Mar 10, 2023
View reviewed changes
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.

docs/static/assets/delete-iam.cast Outdated
@@ -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
Reviewers

@m1ghtym0 m1ghtym0 m1ghtym0 approved these changes

@3u13r 3u13r 3u13r left review comments

@thomasten thomasten thomasten approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
v2.7.0
Development

Successfully merging this pull request may close these issues.
6 participants
@datosh @msanft @m1ghtym0 @3u13r @thomasten @derpsteb
0