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

scenarios, rework top-level page #934

Merged
merged 7 commits into from
Dec 6, 2022
Merged

Conversation

MattDodsonEnglish
Copy link
Contributor

  • Use bulleted description list to list benefits
  • Summarize executors instead of duplicating table
  • Add two scenarios to example, to show it's possible
  • Hide full log output
  • shorten options slug, and fix all references
  • more meaningful headings

Early work for #808

- Use bulleted description list to list benefits
- Summarize executors instead of duplicating table
- Add two scenarios to example, to show it's possible
- Hide full log output
- shorten options slug, and fix all references

Early work for #808
@MattDodsonEnglish
Copy link
Contributor Author

I don't think these edits are controversial, but I'm tagging you, @javaducky , because I did make some small assumptions about how scenarios work and their benefits, and you, @ppcano , because this a small info-architecture change, as it modifies the entrypoint to broader scenario docs.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

There's a version of the docs published here:

https://mdr-ci.staging.k6.io/docs/refs/pull/934/merge

It will be deleted automatically in 30 days.

Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

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

Looks good other than you have confusion between scenario-example.js and example-scenario.js usage. I've added replacements if you were going with scenario-example.js since that appeared as your initial desired name (I'm fine with either name as long as it's applied consistently 😸).

MattDodsonEnglish and others added 5 commits December 5, 2022 14:40
…s.md

Co-authored-by: Paul Balogh <javaducky@gmail.com>
…s.md

Co-authored-by: Paul Balogh <javaducky@gmail.com>
…s.md

Co-authored-by: Paul Balogh <javaducky@gmail.com>
…s.md

Co-authored-by: Paul Balogh <javaducky@gmail.com>
…s.md

Co-authored-by: Paul Balogh <javaducky@gmail.com>
@MattDodsonEnglish
Copy link
Contributor Author

Thanks Paul! You have an eye for detail that I really should develop in myself 😆 .

Copy link
Contributor

@javaducky javaducky left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@ppcano ppcano left a comment

Choose a reason for hiding this comment

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

@MattDodsonEnglish added a suggestion and approved it.


<CodeGroup labels={["scenario example output"]} lineNumbers={[false]}>

```bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
```bash
```javascript

Copy link
Contributor Author

@MattDodsonEnglish MattDodsonEnglish Dec 6, 2022

Choose a reason for hiding this comment

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

@ppcano , just wondering, what's your preference for Javascript over bash to highlight text output? Originally, I was going to use txt, which would have rendered the text all white, then switched to bash since it highlights the number values.

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MattDodsonEnglish sorry. This should be bash. My suggestion was for scenario-example.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh that's a good catch anyway, thanks! Fixed with: bd01aed

@MattDodsonEnglish MattDodsonEnglish merged commit a7d2866 into main Dec 6, 2022
@MattDodsonEnglish MattDodsonEnglish deleted the simplify-scenario branch December 6, 2022 12:19
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.

3 participants