-
Notifications
You must be signed in to change notification settings - Fork 3
fix: add baseURL default #164
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
Conversation
Create a central consts.ts file with DEFAULT_BASE_URL constant and use it as the default value in StackOneToolSet constructor. This allows users to use the SDK without explicitly providing a baseUrl. Closes #149
4f11124 to
4020632
Compare
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR centralizes the default StackOne API base URL by creating a DEFAULT_BASE_URL constant and using it consistently across the codebase. This allows users to instantiate the SDK without explicitly providing a baseUrl parameter.
Key changes:
- Created a new constants file with
DEFAULT_BASE_URL = 'https://api.stackone.com' - Updated
StackOneToolSetconstructor to use a fallback chain:config?.baseUrl || env.STACKONE_BASE_URL || DEFAULT_BASE_URL - Replaced hardcoded URL strings in
createFeedbackToolwith the constant - Exported the constant from the main index file for public use
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/consts.ts | New file defining the DEFAULT_BASE_URL constant for the StackOne API |
| src/toolsets/stackone.ts | Updated to import and use DEFAULT_BASE_URL as the final fallback in the baseUrl resolution chain |
| src/tools/feedback.ts | Replaced hardcoded 'https://api.stackone.com' strings with DEFAULT_BASE_URL constant |
| src/index.ts | Exported DEFAULT_BASE_URL to make it available for public use |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Default base URL for StackOne API | ||
| */ | ||
| export const DEFAULT_BASE_URL = 'https://api.stackone.com'; |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new file is named consts.ts, but there's already an existing constants.ts file in the same directory (containing DEFAULT_HYBRID_ALPHA). For consistency, consider either:
- Adding
DEFAULT_BASE_URLto the existingconstants.tsfile instead of creating a newconsts.tsfile, or - Renaming this file to match the existing
constants.tsnaming convention
This would maintain a consistent naming pattern across the codebase.
| /** | |
| * Default base URL for StackOne API | |
| */ | |
| export const DEFAULT_BASE_URL = 'https://api.stackone.com'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply the first solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 4 files
|
@ryoppippi I've opened a new pull request, #165, to work on those changes. Once the pull request is ready, I'll request review from you. |
glebedel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- Move DEFAULT_HYBRID_ALPHA from constants.ts to consts.ts - Update import in tool.ts to use consts module - Export DEFAULT_HYBRID_ALPHA from index.ts for consistency - Remove redundant constants.ts file This improves consistency by having all constants in a single file rather than split across constants.ts and consts.ts.
Create a central consts.ts file with DEFAULT_BASE_URL constant and use it as the default value in StackOneToolSet constructor. This allows users to use the SDK without explicitly providing a baseUrl.
Closes #149
Summary by cubic
Set a default base URL for the StackOne API so the SDK works without passing baseUrl. Centralized DEFAULT_BASE_URL and applied it across the toolset and feedback tool, with support for env var override.
Bug Fixes
Refactors
Written for commit 6480c00. Summary will update automatically on new commits.