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.

Sign up for GitHub

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

Jump to bottom

feat: exercise limits for read session #27

Merged

infiniteregrets merged 7 commits into main from read-improvements

Nov 20, 2024

Member

infiniteregrets commented Nov 20, 2024

No description provided.

infiniteregrets added 2 commits

November 19, 2024 17:43

..

d1f2c49


          feat: exercise limits for read session

623cac5

infiniteregrets requested a review from a team as a code owner

November 20, 2024 04:04

b3b88a9

shikhar reviewed

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

cr

2fed1da

shikhar reviewed

View reviewed changes

src/main.rs Outdated

@@ @@ -224,9 +225,11 @@ enum StreamActions { @@
                       output: Option<RecordsOut>,
                       /// Limit the number of records returned.
+                      #[arg(long)]

Member

shikhar Nov 20, 2024 •

edited

Loading

can we have -n for this as well?

shikhar reviewed

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

shikhar reviewed

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

e7c9b5c

infiniteregrets commented

View reviewed changes

src/error.rs Show resolved Hide resolved

shikhar reviewed

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

shikhar reviewed

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

shikhar reviewed

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

shikhar reviewed

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

shikhar approved these changes

View reviewed changes

..

2f6d05f

Member

shikhar commented Nov 20, 2024

Nit: Can you make sure the char short is consistently explicit?

Member Author

infiniteregrets commented Nov 20, 2024

Nit: Can you make sure the char short is consistently explicit?

oh is it because clap might panic if it automatically assigns the name? i.e. two commands having the same short arg


          explicit

1d1aff6

Member Author

infiniteregrets commented Nov 20, 2024

@shikhar just making short args explicit for Stream, ill tic for revisiting the rest

Member

shikhar commented Nov 20, 2024 •

edited

Loading

oh is it because clap might panic if it automatically assigns the name? i.e. two commands having the same short arg

IMO just helps a bit to make sure we have the ergonomics in mind when adding args

infiniteregrets merged commit e260538 into main

2 checks passed

infiniteregrets deleted the read-improvements branch

November 20, 2024 16:13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet