Warnings about options.cloud

[olegbespalov]

What?

These changes aim to start promoting the options.cloud. It's the state when it's safe to say to customers that it's safe to use.

These changes were originally part of #3348, but there is a chance that we could merge #3348, but we won't be ready to start offering usage of the options.cloud.

Why?

Reduce the scope and try to make #3348 mergable.

Checklist

• Check the status Add k6 new subcommand #3394 that could bring additional usage of the options.ext.loadimapct and adjust this PR if need
• Ensure that we can promote the options.cloud (backend ready)
• I have performed a self-review of my code.
• I have added tests for my changes.
• I have run linter locally (make lint) and all checks pass.
• I have run tests locally (make tests) and all tests pass.
• I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

• Introducing options.cloud #3348
• Add k6 new subcommand #3394

[codebien]

Blocking as we will have to change the part related to #3394 when it is merged.

But, at this point, I would like to make another attempt in considering to directly introduce the block for the cloud here.

Now that we have this PR, @olegbespalov @andrewslotin do you still want to introduce the block there? I think this PR has a limited scope that fits the case.

[olegbespalov]

If the question is Do you still want to use options.ext.loadimpact in #3394? then I do believe that using a legacy block is still the right thing since it guarantees atomically of the changes.

All parties are also aware of this problem of syncing between these two PRs, and we will do the right things depending on the pace.

I'm not sure this PR should be considered blocked, but I'm not against keeping it like that to stress the importance of some actions that could happen once the #3394 is merged. Also, I'm putting a note to the PR's checklist to emphasize the importance of checking.

[codebien]

These changes were originally part of #3348, but there is a chance that we could merge #3348, but we won't be ready to start offering usage of the options.cloud.

I took the assumption that this PR was already blocked from the quoted condition to be verified. Isn't that? Do you intend to merge it without being sure that we are going to offer the usage of the option? Then eventually revert only this PR?

[olegbespalov]

I took the assumption that this PR was already blocked from the quoted condition to be verified. Isn't that? Do you intend to merge it without being sure that we are going to offer the usage of the option? Then eventually revert only this PR?

It's blocked because of that. That's true. Let me be explicit in the PR body.

But you wrote above:

Blocking as we will have to change the part related to #3394 when it is merged.

That sentence I was answering when I said that I don't agree that it's blocked.

[olegbespalov]

Also, I converted it to a daft since it waits for some internal decisions.

[codecov-commenter]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (b5a6feb) 73.55% compared to head (03fdf37) 73.48%.

❗ Current head 03fdf37 differs from pull request most recent head f35edf2. Consider uploading reports for the commit f35edf2 to get more accurate results

Files	Patch %	Lines
cmd/login_cloud.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3407      +/-   ##
==========================================
- Coverage   73.55%   73.48%   -0.07%     
==========================================
  Files         277      275       -2     
  Lines       20228    20229       +1     
==========================================
- Hits        14879    14866      -13     
- Misses       4401     4409       +8     
- Partials      948      954       +6     

Flag	Coverage Δ
macos	?
ubuntu	73.48% <70.00%> (+0.01%)	⬆️
windows	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mstoykov]

I do wonder if we should do this in the same version we introduce it.

Maybe have a version with it and let us see if there aren't any bugs will be better.

This will also let us go update it ... everywhere - which will be a major problem IMO with all of our documentation and examples

[codebien]

Maybe have a version with it and let us see if there aren't any bugs will be better.

Any bug on this specific feature will require us to hotfix. Plus, on the other side, I also see the risk that if we don't promote it then we may not get exposure and the users will not hit any eventual bugs.
So, considering that it isn't a blocker, and in any case, we have to fix it fast, I'm for just merging it.

[oleiade]

I align with @codebien ☝🏻 and think we should probably just merge it to get it some visibility indeed 🙇🏻

The base branch was changed.