Skip to content
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

Switching from cli@v2 to cli@v3 #30703

Open
karalabe opened this issue Oct 31, 2024 · 0 comments
Open

Switching from cli@v2 to cli@v3 #30703

karalabe opened this issue Oct 31, 2024 · 0 comments

Comments

@karalabe
Copy link
Member

karalabe commented Oct 31, 2024

First up, CLI v3 is not yet released, it's in alpha. I guess the API might change in between, so unsure about this aspect. We're not using that many crazy stuff and upgrading in my tests was faily straightforward so I don't expect much to break, but it's always a risk.

Obviously we can yolo switch in one big PR, but there's a bunch of changes we could do now still using the v2 which would prep our code for a lighter diff when flipping the version:

  • Turn all int flags into cli.Uint64Flag
    • In v3, there are only cli.IntFlag and cli.Uint flag, both 64 bits. We could create some custom flag types for ints, but doens't make much sense really to stray off the std lib on such a basic functionality. By doing the change now, we could reduce the future diff to simple cutting off the 64 suffix.
  • Drop Int flags, enforced via a linter.
    • But realistically, they are all the same, just different names. We don't have any flags that can go negative, so might as well be useful to enforce Uint64Flag across our code via a linter.
  • Maybe drop TextMarshallerFlag.
    • I've ported all the other custom flag types to v3 cleanly (v3 uses generics), but this text marshaller blew up internally with a nil panic inside the cli lib. This might be because v3 has 5 generic methods you need to implement in fancy ways. No idea, but if we only use it for 1 flag (syncmode) and 1 read of it; it might just not be worth to have a custom type (e.g. we inlined the handling for gcmode, which is the same thing).
  • Old "action" method took one param (ctx *cli.Context). They renamed this struct to cli.Command (making it bigger, also removed App), but also added context.Context. This means the ctx arg name will be very unsuitable/clash in v3. So we might do one more prep step of renaming that to cmd in anticipation.
  • v3 suppors string maps, so we can replace out influxdb tags flag with that vs custom code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants
@karalabe and others