-
Notifications
You must be signed in to change notification settings - Fork 3
refactor(examples): replace ACCOUNT_IDS constant with inline placeholders #198
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
…ders Remove the centralised constants.ts file containing hardcoded account IDs and replace with inline placeholder values in each example file. This change addresses feedback from PR #148 review by @glebedel: - Account IDs should not be stored in shared constants - Examples are clearer with inline values that users replace directly - Avoids exposing internal implementation details Changes: - Delete examples/constants.ts with hardcoded account IDs - Update all example files to use inline placeholder accountId variables - Each example now has a clear comment: "Replace with your actual account ID" - Update README.md to reflect new pattern and remove constants.ts references Closes #196
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.
No issues found across 8 files
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 refactors the examples to remove a centralized constants file containing hardcoded account IDs in favor of inline placeholder values. This improves clarity by having users replace account IDs directly in each example file they use, rather than maintaining a separate constants file.
Key changes:
- Removed the centralized
examples/constants.tsfile with hardcoded account IDs - Added inline placeholder account ID variables with clear replacement instructions in each example
- Updated documentation to reflect the new inline pattern
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| examples/constants.ts | Deleted centralized constants file containing hardcoded account IDs |
| examples/planning.ts | Replaced ACCOUNT_IDS imports with inline atsAccountId and hrisAccountId placeholders |
| examples/openai-integration.ts | Replaced ACCOUNT_IDS.HRIS with inline accountId placeholder |
| examples/meta-tools.ts | Replaced ACCOUNT_IDS.HRIS with inline accountId placeholder |
| examples/index.ts | Replaced ACCOUNT_IDS.HRIS with inline accountId placeholder |
| examples/human-in-the-loop.ts | Replaced ACCOUNT_IDS.HRIS with inline accountId placeholder |
| examples/ai-sdk-integration.ts | Replaced ACCOUNT_IDS.HRIS with inline accountId placeholder |
| examples/README.md | Updated documentation to explain inline placeholder pattern and removed constants.ts references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Summary
Remove the centralised
constants.tsfile containing hardcoded account IDs and replace with inline placeholder values in each example file.This change addresses feedback from PR #148 review by @glebedel:
Changes
examples/constants.tswith hardcoded account IDsaccountIdvariablesREADME.mdto reflect new pattern and removeconstants.tsreferencesTest plan
Closes #196
Summary by cubic
Replaced the shared ACCOUNT_IDS constant with inline accountId placeholders in each example to simplify setup and avoid shipping hardcoded IDs.
Refactors
Migration
Written for commit 753a137. Summary will update automatically on new commits.