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

cmd/paratime/show: Add show events command #333

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented Dec 10, 2024

Useful for easily fetching all events in a paratime block. The existing paratime show displays only block events (when querying info about a block) or events of a single transaction (when querying info about a transaction within a block).

Alternative would be updating the existing paratime show <round> to return all events emitted in a block, but the number of events could be large, so it might bloat the output for someone not really interested about this.

Copy link

netlify bot commented Dec 10, 2024

Deploy Preview for oasisprotocol-cli canceled.

Name Link
🔨 Latest commit f248ddb
🔍 Latest deploy log https://app.netlify.com/sites/oasisprotocol-cli/deploys/675872569cf9270008675fb9

@ptrus ptrus force-pushed the ptrus/feature/paratime-show-events branch 2 times, most recently from fda3032 to d1fdd86 Compare December 10, 2024 16:13
var (
outputFormat string
selectedRound uint64

showCmd = &cobra.Command{
Use: "show { <round> [ <tx-index> | <tx-hash> ] | parameters }",
Short: "Show information about a ParaTime block, its transactions or other parameters",
Use: "show { <round> [ <tx-index> | <tx-hash> ] | parameters | events }",
Copy link
Member Author

Choose a reason for hiding this comment

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

These commands are currently inconsistent regarding the round argument. paratime show parameters uses --round flag, while paratme show <round> uses round as a positional argument.

show events is consistent with show parameters since that was the easiest to implement. But this should probably be cleaned up (in a followup).

Copy link
Member

Choose a reason for hiding this comment

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

Initially it was not meant to show other things, so if we now have parameters and events, we should maybe also have show block or similar.

Copy link
Member

Choose a reason for hiding this comment

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

The initial idea was to resemble oasis network show. So try to autodetect what user wanted with as little typing as possible and then offer more explicit variants, if needed.

Some alternative ideas:

  • show <round> --events
  • show events:<round>
  • show <round>:events
  • or simply show all transactions with events when calling show <round>, also pass --format json and let user filter them with jq

IMO the current implementation makes the most sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll merge it as is for now.

@ptrus ptrus force-pushed the ptrus/feature/paratime-show-events branch from d1fdd86 to eb5ab35 Compare December 10, 2024 16:31
@ptrus ptrus force-pushed the ptrus/feature/paratime-show-events branch from eb5ab35 to f248ddb Compare December 10, 2024 16:54
@ptrus ptrus marked this pull request as ready for review December 12, 2024 12:17
@ptrus ptrus requested review from kostko and matevz December 12, 2024 15:31
@ptrus ptrus merged commit 59f03ea into master Dec 13, 2024
4 checks passed
@ptrus ptrus deleted the ptrus/feature/paratime-show-events branch December 13, 2024 14:28
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.

3 participants