Skip to content

Conversation

@shota3506
Copy link
Contributor

Replace the experimental golang.org/x/exp/maps package with the standard library maps package

Motivation

  • Use stable standard library APIs
  • Reduce dependency on experimental packages

Modifications

  • use standard maps and slices package instead of experimental package
  • update go.mod

Verification

Documentation

@shota3506 shota3506 force-pushed the chore/use-standard-maps-package branch from 635b6e3 to c480ffb Compare October 20, 2025 07:48
@Joibel
Copy link
Member

Joibel commented Oct 20, 2025

Thanks @shota3506 - is your aim to run through all of modernize? If so, once you're done I'd also appreciate a PR to run modernize as part of linting.

@shota3506
Copy link
Contributor Author

shota3506 commented Oct 20, 2025

hi @Joibel
As far as I know, migrating from the exp package to the std package is not within the scope of gopls modernize for now.

But yes, I agree it would be a great idea to run modernize in linting. Would it be okay if I do that as a separate task?

@shota3506
Copy link
Contributor Author

Let me add modernize analyzer after next golangci-lint minor version release
golangci/golangci-lint#6126

@Joibel
Copy link
Member

Joibel commented Oct 20, 2025

Let me add modernize analyzer after next golangci-lint minor version release golangci/golangci-lint#6126

Oh, that's nice that it's being added there, hadn't seen that.

@Joibel
Copy link
Member

Joibel commented Oct 20, 2025

hi @Joibel ✋ As far as I know, migrating from the exp package to the std package is not within the scope of gopls modernize for now.

But yes, I agree it would be a great idea to run modernize in linting. Would it be okay if I do that as a separate task?

Separate task would be best, yes please. Possibly split up some of the fixes it provides from actually enabling it in linting too, just so the PRs are more reviewable.

@shota3506
Copy link
Contributor Author

@Joibel Okay, I’ll open some PRs to address modernize alerts in the existing codebase before enabling it in lint.

This PR is independent of that, so please review it when you have a moment. (I assume the CI errors are unrelated to these changes?)

Copy link
Member

@Joibel Joibel left a comment

Choose a reason for hiding this comment

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

Thanks for working on this

@Joibel Joibel enabled auto-merge (squash) October 22, 2025 09:55
auto-merge was automatically disabled October 23, 2025 03:32

Head branch was pushed to by a user without write access

@Joibel Joibel merged commit 0cc3b2c into argoproj:main Oct 23, 2025
39 checks passed
ItielOlenick pushed a commit to ItielOlenick/argo-workflows that referenced this pull request Oct 26, 2025
…ge (argoproj#14943)

Signed-off-by: shota3506 <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Co-authored-by: Alan Clucas <[email protected]>
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.

2 participants