-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Refactor new
command
#4153
base: master
Are you sure you want to change the base?
Refactor new
command
#4153
Conversation
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Happy to fix the linter things if we feel this approach makes sense 👍 |
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
Signed-off-by: Daniel González Lopes <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍🏻 🚀
Honestly, I'd be happy for us to move forward with this. Have you considered moving back to an engineering career Daniel ? 🙉
# Create a minimal k6 script in the current directory. By default, k6 creates script.js. | ||
{{.}} new | ||
exampleText := getExampleText(c.gs, ` | ||
# Create a minimal k6 script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed that both k6 new
and k6 new --template minimal
work, and it looks like it generates the same script file. As a result I would keep documenting an example of the default behavior of k6 new
here 🙇🏻
Also as we're at it, it wasn't immediatly obvious to me that the k6 new
command supports a file argument (not flag) from the examples, we might want to also add one demonstrating that?
} | ||
} | ||
|
||
export default async function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really a nitpick, and a personal preference, but I would find the code more readable with actual newlines 😄
Early draft PR. Issue with more context: #4154
What?
Why?
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)