Skip to content

Conversation

@codebien
Copy link
Contributor

@codebien codebien commented Apr 18, 2025

What?

It merges the launcher package into internal/cmd.

Why?

At the moment, k6 isn't designed to have a separate package for startup ops, and doing so was causing problems. So, as we want to avoid major refactoring, then it necessitates adhering to this structure.
This is especially important for the config, which is internal to the 'cmd' package. While exporting a dedicated 'config' package would be beneficial, it would require significant time and effort. For now, maintaining the current structure will be simple, and simplicity is key here.

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

@codebien codebien self-assigned this Apr 18, 2025
@codebien codebien force-pushed the binpro/move-launcher branch from f30816b to b4c053c Compare April 18, 2025 19:44
@codebien codebien marked this pull request as ready for review April 18, 2025 20:16
@codebien codebien requested a review from a team as a code owner April 18, 2025 20:16
@codebien codebien requested review from inancgumus and removed request for a team and inancgumus April 18, 2025 20:16
@codebien codebien added this to the v1.0.0 milestone Apr 18, 2025
Copy link
Contributor

@joanlopez joanlopez 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! 👍🏻

Arguably, this is yet another small reason to use something along the lines of "executor" for #4708, as it is mainly called from the public function Execute() and the execute.go file.

@codebien codebien force-pushed the binpro/move-launcher branch from b4c053c to 6a7a0de Compare April 23, 2025 10:56
@codebien codebien changed the base branch from binpro/rename-runners to feat/binary-provisioning April 23, 2025 10:58
@codebien codebien force-pushed the binpro/move-launcher branch from 6a7a0de to 2a5e97b Compare April 23, 2025 11:12
@codebien codebien merged commit 2c3c934 into feat/binary-provisioning Apr 23, 2025
28 checks passed
@codebien codebien deleted the binpro/move-launcher branch April 23, 2025 12:33
@codebien codebien mentioned this pull request Apr 23, 2025
14 tasks
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.

4 participants