Skip to content

Conversation

@HadrienPatte
Copy link
Contributor

  • Have you signed the CLA?

Use modern Go constructs:

  • replace interface{} with any (available since go1.18)
  • replace 3-clause loop with range loop (available since go1.22)
  • use slices package (available since go1.21)

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Command Mean [s] Min [s] Max [s] Relative
BASE 8.504 ± 0.021 8.477 8.546 1.00
HEAD 8.509 ± 0.020 8.469 8.543 1.00 ± 0.00

@letFunny
Copy link
Collaborator

@HadrienPatte Thanks for this and apologies for not reviewing it. At the moment we are focused on delivering some major features that will significantly improve the UX. Once we are done with those we will prioritize reviewing and merging this PR.

@reneleonhardt
Copy link

Seems odd, upgrading go to 1.23 in the meantime without merging all those nice modernizations first...

@HadrienPatte
Copy link
Contributor Author

@canonical/rocks gentle bump on this PR ;)

Copy link
Collaborator

@letFunny letFunny 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 this PR and apologies for taking long to review. Even though it looks simple, every change to slices.Delete and slices.Clone has to be inspected manually because they have slightly different semantics than the code that was written before them (i.e. they now respect nilness).

While this is approved, it will still remain low priority so it might get picked up in the next couple of weeks. Thanks again for contributing!

@letFunny
Copy link
Collaborator

letFunny commented Sep 5, 2025

@HadrienPatte can you push an empty commit git commit --allow-empty ... to trigger the CI, there seems to be a bug in Github.

@letFunny letFunny added Simple Nice for a quick look on a minute or two Polish Refactorings, etc labels Sep 12, 2025
Copy link
Contributor

@niemeyer niemeyer 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 these!

@niemeyer niemeyer merged commit aca3b32 into canonical:main Sep 15, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Polish Refactorings, etc Simple Nice for a quick look on a minute or two

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants