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

[Bug] Decide on --name vs --type and use consistently. #475

Closed
dandavison opened this issue Feb 24, 2024 · 3 comments · Fixed by #580
Closed

[Bug] Decide on --name vs --type and use consistently. #475

dandavison opened this issue Feb 24, 2024 · 3 comments · Fixed by #580
Labels
bug Something isn't working

Comments

@dandavison
Copy link
Contributor

dandavison commented Feb 24, 2024

From #455 (comment)

In our current (0.x) temporal CLI, signal and update use --name whereas query uses --type. However, IMO we need the flag name to be the same for signal, query, and update, so a compatibility break with 0.x is inevitable.

@dandavison dandavison added the bug Something isn't working label Feb 24, 2024
@lorensr
Copy link
Contributor

lorensr commented Feb 25, 2024

Current CLI matches the gRPC API terminology.

Docs uses name for both Query and Signal, nothing for Update yet.

Inclined toward using name for all 3 in docs & CLI.

@cretz
Copy link
Member

cretz commented Feb 26, 2024

Inclined toward using name for all 3 in docs & CLI.

Same and we've done this for newer SDKs. We would have to decide this now pre-1.0. Query is pretty commonly used so this would be our biggest compatibility break to date I believe, but at least (with --type removed) it'd be an immediately CLI flag validation failure. Alternatively we could deprecate --type in favor of --name, but we'd have to admit to ourselves then we'd never remove it if we're not willing to remove it pre-1.0.

@Sushisource
Copy link
Member

We could make them aliases for each other potentially.

Personally I think type is slightly more accurate, but, honestly either works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants