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

improve logging and default handling #8

Merged
merged 2 commits into from
Feb 6, 2025
Merged

improve logging and default handling #8

merged 2 commits into from
Feb 6, 2025

Conversation

ianmacartney
Copy link
Collaborator

Only print help when it's being called directly from the CLI /dashboard, without modifying other flows.
When it's called from the component, cursor is a string or null, dryRun is always a boolean.
From the CLI / dashboard, they won't be set by default (though the dashboard used to set defaults like cursor: "" and batchSize: 0 so we also check for that in case it comes back).


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ianmacartney ianmacartney requested a review from ldanilek January 24, 2025 22:34
@ianmacartney
Copy link
Collaborator Author

ianmacartney commented Jan 24, 2025

@ldanilek would you mind sanity checking my logic here?
@convex-dev/[email protected] if you want to test on your own code

Copy link

@ldanilek ldanilek left a comment

Choose a reason for hiding this comment

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

looks good to me

if (args.cursor === "" || args.cursor === undefined) {
if (args.dryRun) {

const numItems = args.batchSize || defaultBatchSize;
Copy link

Choose a reason for hiding this comment

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

i think batchSize=0 still becomes defaultBatchSize. is that intentional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not necessary, but is also ok here. I believe this is only possible when calling from the dashboard / CLI and that value is likely only ever 0 if it's set as a "helpful" default.

@ianmacartney ianmacartney merged commit 7d82e47 into main Feb 6, 2025
@ianmacartney ianmacartney deleted the ian/logging branch February 6, 2025 08:48
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.

2 participants