Skip to content

Add copyloopvar as a linter and auto fix #4608

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 15, 2025
Merged

Add copyloopvar as a linter and auto fix #4608

merged 1 commit into from
May 15, 2025

Conversation

mstoykov
Copy link
Contributor

@mstoykov mstoykov commented Mar 6, 2025

What?

Add copyloopvar as a linter and autofix its issues

Why?

This removes pre 1.22 copying of values in a lot of tests with table tests and make them more modern and slightly shorter.

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 k6-documentation: grafana/k6-docs#PR-NUMBER
  • I have updated the TypeScript definitions: grafana/k6-DefinitelyTyped#PR-NUMBER
  • I have updated the release notes: link

Related PR(s)/Issue(s)

@mstoykov mstoykov added this to the v1.0.0-rc1 milestone Mar 6, 2025
@mstoykov mstoykov requested a review from a team as a code owner March 6, 2025 12:06
@mstoykov mstoykov requested review from oleiade and joanlopez and removed request for a team March 6, 2025 12:06
joanlopez
joanlopez previously approved these changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@joanlopez joanlopez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm not quite sure we really want the new lines it added in a few cases, but that's not very relevant, so it's alright! 👌🏻

oleiade
oleiade previously approved these changes Mar 7, 2025
View reviewed changes
@mstoykov mstoykov force-pushed the BumpGoVersion branch 2 times, most recently from d3dfc08 to c2c1d72 Compare March 7, 2025 14:52
Base automatically changed from BumpGoVersion to master March 10, 2025 08:31
@mstoykov mstoykov dismissed stale reviews from oleiade and joanlopez March 10, 2025 08:31

The base branch was changed.

@mstoykov mstoykov modified the milestones: v1.0.0-rc1, v1.0.0 Mar 17, 2025
@mstoykov
Copy link
Contributor Author

mstoykov commented Mar 17, 2025 •
edited
Loading

I am not going to prioritize this for this cycle. The linter isn't fixing all cases, so there is more manual work than expected.

All done now

@codebien codebien modified the milestones: v1.0.0, v1.1.0 May 6, 2025
@mstoykov mstoykov requested review from oleiade and joanlopez May 8, 2025 13:42
@joanlopez
Copy link
Contributor

I am not going to prioritize this for this cycle. The linter isn't fixing all cases, so there is more manual work than expected.

Is it fixed now? Or there's still manual work remaining? 🤔

@mstoykov
Copy link
Contributor Author

@joanlopez this was for hte previous cycle, as I started it after golangci update , but as I mentioned the autofixer didn't fix a lot of the cases, so I had to go do them manually.

But it is all done now

@joanlopez
Copy link
Contributor

But it is all done now

Great! 👏🏻

ankur22
ankur22 approved these changes May 15, 2025
View reviewed changes
Copy link
Contributor

@ankur22 ankur22 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@mstoykov mstoykov merged commit f3089d5 into master May 15, 2025
33 checks passed
@mstoykov mstoykov deleted the copyloopvar branch May 15, 2025 08:29
@BrewTestBot BrewTestBot mentioned this pull request Jun 25, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ankur22 ankur22 ankur22 approved these changes

@joanlopez joanlopez joanlopez approved these changes

@oleiade oleiade Awaiting requested review from oleiade

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@mstoykov @joanlopez @oleiade @ankur22 @codebien