-
Notifications
You must be signed in to change notification settings - Fork 0
Update URL, Edge support #3
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
Caution Review failedThe pull request is closed. WalkthroughThe updates replace all references to the old domain with the new "capture.page" domain across documentation, tests, and code. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CaptureClass as Capture
participant API as capture.page
participant EdgeAPI as edge.capture.page
User->>CaptureClass: new Capture(apiKey, secret, { useEdge: true/false })
Note right of CaptureClass: useEdge determines base URL
User->>CaptureClass: buildImageUrl(...)
alt useEdge is true
CaptureClass->>EdgeAPI: Generate URL with edge.capture.page
else useEdge is false
CaptureClass->>API: Generate URL with capture.page
end
CaptureClass-->>User: Return constructed URL
Possibly related PRs
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
5-5
: Fix bare URLs in MarkdownThe URLs on lines 5 and 15 should be properly formatted as Markdown links rather than bare URLs.
-Get your API Key and Secret from https://capture.page/console +Get your API Key and Secret from [https://capture.page/console](https://capture.page/console) -List of all capture options: https://docs.capture.page +List of all capture options: [https://docs.capture.page](https://docs.capture.page)Also applies to: 15-15
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Bare URL used
null(MD034, no-bare-urls)
1-92
: Add documentation for the newuseEdge
optionThe README has been updated with the new domain, but it doesn't include documentation for the new
useEdge
option that was added. Consider adding an example showing how to initialize with this option.Something like:
// Initialize with edge domain support const capture = new Capture(YOUR_API_KEY, YOUR_API_SECRET, { useEdge: true });🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Bare URL used
null(MD034, no-bare-urls)
9-9: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
15-15: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md
(6 hunks)__tests__/test.ts
(4 hunks)package.json
(1 hunks)src/index.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
__tests__/test.ts (1)
src/index.ts (1)
Capture
(7-130)
🪛 markdownlint-cli2 (0.17.2)
README.md
5-5: Bare URL used
null
(MD034, no-bare-urls)
15-15: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (8)
package.json (1)
4-4
: Description update looks goodThe description has been updated to reflect the new domain name, which is consistent with the broader changes in this PR.
__tests__/test.ts (3)
16-17
: Domain updates in test expectations look goodAll test assertions have been properly updated to use the new
capture.page
domain instead of the previous domain.Also applies to: 26-27, 40-41, 50-51, 64-65, 78-79
21-21
: Domain updates in test samples look goodThe test samples properly use the new domain in the URL examples being tested.
Also applies to: 45-45
84-106
: Good test coverage for the new Edge featureExcellent addition of tests for the new
useEdge
option, verifying that all URL building methods correctly use the edge domain when this option is enabled.README.md (1)
3-5
: Domain updates in documentation look goodAll references to the old domain have been properly updated to use the new
capture.page
domain in the documentation and examples.Also applies to: 15-15, 25-25, 33-33, 49-49, 57-57, 72-72, 76-76, 87-87, 91-91
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Bare URL used
null(MD034, no-bare-urls)
src/index.ts (3)
8-9
: Base URL updates look goodThe API URL has been properly updated to the new domain, and a new EDGE_URL constant has been added.
13-13
: Edge support in class properties and constructor looks goodThe
options
property and constructor have been properly updated to support the newuseEdge
flag, with a sensible default offalse
.Also applies to: 15-15, 18-18
80-80
: URL building with edge support looks goodThe
_buildUrl
method now dynamically selects between the regular API URL and the edge URL based on theuseEdge
option.
Summary by CodeRabbit
New Features
Documentation
Tests