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

V1 Documentation Updates #40

Merged
merged 12 commits into from
Sep 1, 2020
Merged

V1 Documentation Updates #40

merged 12 commits into from
Sep 1, 2020

Conversation

ncatelli
Copy link
Contributor

@ncatelli ncatelli commented Aug 28, 2020

Summary

This PR includes documentation updates for the 1.0 major version bump including new gifs showing functionality.

All gif and documentation changes attempt to replicate the previous demonstrations as closely as possible, primarily making changes to accommodate any differences to the UX in v1.

Still To Fix

  • This PR does not include fixes to failing tests that existed within v1.

@rothgar
Copy link
Collaborator

rothgar commented Aug 28, 2020

Would you mind making the gif dimensions smaller (some of them are huge) and the text bigger? For a demo it may also be a good idea to always use the long form of a flag (e.g. --filter instead of -f) and speed up the recording by removing frames with something like gifsicle https://graphicdesign.stackexchange.com/a/20913

@@ -28,7 +28,7 @@ To use a specific profile and/or region, use the `-p (--profiles)` or `-r (--reg

e.g. `-p account1,account2 -r us-east-1,us-west-2`

Take careful note that if multiple regions *and* profiles are specified, `ssm-run` will return results for instances across all the possible permutations.
Take careful note that if multiple regions *and* profiles are specified, `ssm-sessionb` will return results for instances across all the possible permutations.
Copy link
Contributor

Choose a reason for hiding this comment

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

ssm-sessionb -> ssm session?

INFO Instance ID Region Profile Status
INFO i-12345 us-east-1 profile1 Success
INFO i-67890 us-east-1 profile1 Success
> ./ssm run -p 'profile1' -i 'i-0879fe217fe11ad86,i-062aede5be23eef7a' -c 'uname > /dev/null 2>&1'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we show the output of the command instead of just piping to /dev/null so as to illustrate what that would look like interspersed with the status lines for each instance?

INFO Fetched 1 instances for account [profile1] in [us-east-1].
INFO Instance ID Region Profile Status
INFO i-12345 us-east-1 profile1 Success
> ./ssm run -p 'profile1' -i i-12345 -c 'uname'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to keep the ./ssm in? Feel like that was an artifact from when this was bash script or the binary was written to the root of the project. A local build of the project would be using bin/ssm and an installed version using 'go get' would not need the './' prefix. This comment applies to many other examples.

@sendqueery sendqueery self-requested a review September 1, 2020 14:42
@sendqueery sendqueery merged commit ed7c423 into disneystreaming:v1 Sep 1, 2020
sendqueery pushed a commit that referenced this pull request Sep 3, 2020
* Update ssm run documentation with v1 changes.

* Update ssm session readme with v1 changes.

* Update gifs for v1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants