Skip to content

Commit 4b34dc0

Browse files
committed
refactor: address second round of human review feedback
- Use SortValue type alias in ListFlags instead of inline literal union - Fix MAX_LIMIT JSDoc: removed incorrect claim about internal pagination (listSpans passes per_page directly, no client-side pagination) - Add default params to validateLimit(value, min=1, max=1000) - Change limit brief to (<=) format
1 parent 817eb16 commit 4b34dc0

File tree

2 files changed

+8
-10
lines changed

2 files changed

+8
-10
lines changed

src/commands/span/list.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,25 @@ import {
3434
} from "../../lib/resolve-target.js";
3535
import { validateTraceId } from "../../lib/trace-id.js";
3636

37+
type SortValue = "date" | "duration";
38+
3739
type ListFlags = {
3840
readonly limit: number;
3941
readonly query?: string;
40-
readonly sort: "date" | "duration";
42+
readonly sort: SortValue;
4143
readonly json: boolean;
4244
readonly fresh: boolean;
4345
readonly fields?: string[];
4446
};
4547

46-
type SortValue = "date" | "duration";
47-
4848
/** Accepted values for the --sort flag (matches trace list) */
4949
const VALID_SORT_VALUES: SortValue[] = ["date", "duration"];
5050

5151
/**
5252
* CLI-side upper bound for --limit.
5353
*
54-
* The Sentry Events API (spans dataset) accepts `per_page` up to 100 per
55-
* request, but the CLI allows requesting up to 1000 as a convenience —
56-
* the API client paginates internally when needed. This matches the cap
57-
* used by `issue list`, `trace list`, and `log list`.
54+
* Passed directly as `per_page` to the Sentry Events API (spans dataset).
55+
* Matches the cap used by `issue list`, `trace list`, and `log list`.
5856
*/
5957
const MAX_LIMIT = 1000;
6058

@@ -114,7 +112,7 @@ export function parsePositionalArgs(args: string[]): {
114112
* Parse --limit flag, delegating range validation to shared utility.
115113
*/
116114
function parseLimit(value: string): number {
117-
return validateLimit(value, 1, MAX_LIMIT);
115+
return validateLimit(value, 1, MAX_LIMIT); // min=1 is validateLimit's default, explicit for clarity
118116
}
119117

120118
/**
@@ -210,7 +208,7 @@ export const listCommand = buildCommand({
210208
limit: {
211209
kind: "parsed",
212210
parse: parseLimit,
213-
brief: `Number of spans (1-${MAX_LIMIT})`,
211+
brief: `Number of spans (<=${MAX_LIMIT})`,
214212
default: String(DEFAULT_LIMIT),
215213
},
216214
query: {

src/lib/arg-parsing.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ export function detectSwappedTrialArgs(
176176
* validateLimit("0", 1, 1000) // throws
177177
* validateLimit("abc", 1, 1000) // throws
178178
*/
179-
export function validateLimit(value: string, min: number, max: number): number {
179+
export function validateLimit(value: string, min = 1, max = 1000): number {
180180
const num = Number.parseInt(value, 10);
181181
if (Number.isNaN(num) || num < min || num > max) {
182182
throw new Error(`--limit must be between ${min} and ${max}`);

0 commit comments

Comments
 (0)