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

Add md-k6 script + new workflow #1828

Merged
merged 6 commits into from
Jan 6, 2025
Merged

Add md-k6 script + new workflow #1828

merged 6 commits into from
Jan 6, 2025

Conversation

federicotdn
Copy link
Contributor

@federicotdn federicotdn commented Dec 23, 2024

What?

Adds a new script, md-k6.py, that is run via the run-code-blocks.yml Workflow. The workflow is run only when .md files are modified in the k6/next/** directory via a PR. The script then takes the paths of all .md files that were modified in the PR (in comparison to main) and then run all JS codeblocks within those files. If any of the scripts causes k6 to exit with 1, or logs any type of error, then the workflow will fail.

It is possible to skip the running of the script by adding a magic comment on top of the script: <!--md-k6:skip-->.

Checklist

  • I have used a meaningful title for the PR.
  • I have described the changes I've made in the "What?" section above.
  • I have performed a self-review of my changes.
  • I have run the npm start command locally and verified that the changes look good.
  • I have made my changes in the docs/sources/k6/next folder of the documentation.
  • I have reflected my changes in the docs/sources/k6/v{most_recent_release} folder of the documentation.
  • I have reflected my changes in the relevant folders of the two previous k6 versions of the documentation (if still applicable to previous versions).
  • I have made my changes in the docs/sources/k6/next folder of the documentation.

Related PR(s)/Issue(s)

Better options

Add workflow

Tweak

Fix param

Test MD creation, modification, rename and deletion

More tweaks

Revert "Test MD creation, modification, rename and deletion"

This reverts commit 393424d.

Tweak script

Test file modifications

Compare changes on PR commits only

Revert "Test file modifications"

This reverts commit cab6dae.

More tweaks
@federicotdn federicotdn marked this pull request as ready for review December 31, 2024 10:51
@heitortsergent
Copy link
Collaborator

heitortsergent commented Jan 2, 2025

This is awesome, thanks so much for creating this @federicotdn! 🎉

Tagging @jdbaldry here as well if you'd like to take a look 🤓 . I'm not sure if this could be applicable to other repos, but what do you think of having this in the k6-docs repo?

@federicotdn
Copy link
Contributor Author

I would rather keep it here since this is only documentation-related, but also I have a feeling that adding it to the main k6 repo would take more time in terms of reviewing.

@heitortsergent
Copy link
Collaborator

Ah sorry @federicotdn I just meant adding this to the k6-docs repo, not the k6 main repo. 😅

Copy link
Collaborator

@heitortsergent heitortsergent left a comment

Choose a reason for hiding this comment

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

Made some small edits to the README, thanks for including a new section there! 🙇

I tested this locally running the script on a few files, and it works as expected 🤓 . I think this will be really helpful to keep the doc samples working and up to date!

CONTRIBUTING/README.md Outdated Show resolved Hide resolved
CONTRIBUTING/README.md Outdated Show resolved Hide resolved
@jdbaldry
Copy link
Member

jdbaldry commented Jan 6, 2025

Cool script, thanks for making me aware of it. It's a neat solution to ensuring the validity of code blocks. Another mechanism, used by the Mimir team, is to write tests using the code or configuration files directly and then embed that code or configuration in the documentation using something like embedmd.

@federicotdn
Copy link
Contributor Author

@jdbaldry thanks! And yes, that sounds like a good idea with embedmd. Maybe we could do that in the future, it would require some initial effort for going through all the files (unless we do it progressively). After that we could delete this script and run the k6 .js files directly.

federicotdn and others added 2 commits January 6, 2025 11:22
Co-authored-by: Heitor Tashiro Sergent <[email protected]>
Co-authored-by: Heitor Tashiro Sergent <[email protected]>
@jdbaldry
Copy link
Member

jdbaldry commented Jan 6, 2025

@jdbaldry thanks! And yes, that sounds like a good idea with embedmd. Maybe we could do that in the future, it would require some initial effort for going through all the files (unless we do it progressively). After that we could delete this script and run the k6 .js files directly.

No need to migrate if this is working for you :)

@federicotdn federicotdn merged commit 934a4ac into main Jan 6, 2025
3 checks passed
@federicotdn federicotdn deleted the new-scripts branch January 6, 2025 12:54
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