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

Add checks for mutually-exclusive sf orders ls flags #66

Merged
merged 10 commits into from
Feb 10, 2025

Conversation

Sladuca
Copy link
Contributor

@Sladuca Sladuca commented Feb 8, 2025

  • add explicit checks for mutually-exclusive flags in sf orders ls
  • add to to --public description indicating that --public only includes open orders
  • run deno fmt
  • add CI that runs deno fmt and deno check

Copy link

semanticdiff-com bot commented Feb 8, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  package.json  100% smaller
  src/checkVersion.ts  100% smaller
  src/helpers/config.ts  100% smaller
  src/helpers/errors.ts  100% smaller
  src/helpers/price.ts  100% smaller
  src/helpers/units.ts  100% smaller
  src/helpers/urls.ts  100% smaller
  src/lib/ConfirmInput.tsx  100% smaller
  src/lib/Quote.tsx  100% smaller
  src/lib/clusters/keys.tsx  100% smaller
  src/lib/login.ts  100% smaller
  src/lib/posthog.ts  100% smaller
  src/lib/sell/index.tsx  100% smaller
  src/lib/tokens.ts  100% smaller
  src/lib/buy/index.tsx  100% smaller
  src/lib/sell.ts  100% smaller
  src/lib/updown.tsx  100% smaller
  src/lib/upgrade.ts  99% smaller
  src/lib/orders/OrderDisplay.tsx  99% smaller
  src/lib/dev.ts  99% smaller
  src/scripts/release.ts  98% smaller
  src/lib/clusters/kubeconfig.ts  97% smaller
  src/lib/contracts/ContractDisplay.tsx  96% smaller
  src/index.ts  94% smaller
  src/lib/clusters/kubeconfig.test.ts  93% smaller
  src/lib/contracts/index.tsx  91% smaller
  src/lib/balance.ts  90% smaller
  src/helpers/waitingForOrder.ts  89% smaller
  src/lib/clusters/clusters.tsx  70% smaller
  src/lib/orders/index.tsx  50% smaller
  .github/workflows/ci.yml  0% smaller
  README.md Unsupported file format
  deno.json  0% smaller

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added validation checks for mutually exclusive flags in order listing and improved code formatting across the codebase.

  • Added flag validation in src/lib/orders/index.tsx to prevent incompatible combinations like --public with --only-filled or --only-open with --only-cancelled
  • Updated src/schema.ts to reflect mutually exclusive flag constraints in the API interface for order query parameters
  • Clarified --public flag description to indicate it implies --only-open in order listing
  • Improved code formatting consistency with trailing commas and proper line wrapping across multiple files
  • Enhanced error messages to be more descriptive when incompatible flags are used together

32 file(s) reviewed, 10 comment(s)
Edit PR Review Bot Settings | Greptile

src/lib/ConfirmInput.tsx Show resolved Hide resolved
src/lib/balance.ts Show resolved Hide resolved
src/lib/clusters/clusters.tsx Outdated Show resolved Hide resolved
src/lib/contracts/ContractDisplay.tsx Show resolved Hide resolved
src/lib/contracts/index.tsx Show resolved Hide resolved
src/lib/sell/index.tsx Show resolved Hide resolved
src/lib/sell.ts Show resolved Hide resolved
src/lib/sell.ts Show resolved Hide resolved
src/lib/sell.ts Show resolved Hide resolved
src/lib/updown.tsx Show resolved Hide resolved
Copy link
Member

@sigmachirality sigmachirality left a comment

Choose a reason for hiding this comment

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

Main issue is using commander.js more, otherwise some nits that you can take or leave (ofc I prefer take but otherwise it's whatever)

README.md Show resolved Hide resolved
src/lib/orders/index.tsx Outdated Show resolved Hide resolved
src/lib/sell/index.tsx Show resolved Hide resolved
src/schema.ts Outdated Show resolved Hide resolved
@sigmachirality sigmachirality self-requested a review February 10, 2025 02:46
Copy link
Member

@sigmachirality sigmachirality left a comment

Choose a reason for hiding this comment

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

lgtm!

@sigmachirality sigmachirality changed the title add checks for mutually-exclusive flags, fmt Add checks for mutually-exclusive flags, fmt Feb 10, 2025
@sigmachirality sigmachirality changed the title Add checks for mutually-exclusive flags, fmt Add checks for mutually-exclusive sf orders ls flags Feb 10, 2025
@sigmachirality sigmachirality merged commit 92ed020 into main Feb 10, 2025
1 check passed
@sigmachirality sigmachirality deleted the seb/sf-orders-ls-incompatible-flags branch February 10, 2025 04:12
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