-
Notifications
You must be signed in to change notification settings - Fork 0
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
7: Fix Algolia search crawler to work properly to re-index content #33
Conversation
…ep to install homebrew
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in the pull request involve updates to the workflow configuration for Algolia Search in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub
participant Workflow
participant Scraper
participant Homebrew
User->>GitHub: Push to 7-fix-search
GitHub->>Workflow: Trigger workflow
Workflow->>Homebrew: Install Homebrew
Homebrew-->>Workflow: Installation complete
Workflow->>Scraper: Run scraper
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
⚡️ Lighthouse ReportMobile (view report)
Desktop (view report)
Lighthouse ran on https://uniswap-docs-staging-afrnr0quh-uniswap.vercel.app |
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: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/search.yaml
(1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/search.yaml
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
.github/workflows/search.yaml (2)
6-9
: Ensure temporary trigger is tracked for removal
The branch-specific trigger is marked as temporary with a TODO comment. To prevent this from being forgotten:
- Create a follow-up issue for removing this trigger
- Add the issue number to the TODO comment
Would you like me to help create a GitHub issue to track the removal of this temporary trigger?
Line range hint 19-22
: Verify Algolia configuration and scraper setup
The workflow depends on:
- Valid Algolia credentials in GitHub secrets
- Proper configuration in
config.json
Let's verify these dependencies exist:
🧰 Tools
🪛 actionlint
15-15: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
19-19: shellcheck reported issue in this script: SC2002:style:2:115: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead
(shellcheck)
⚡️ Lighthouse ReportMobile (view report)
Desktop (view report)
Lighthouse ran on https://uniswap-docs-staging-ovxcw7248-uniswap.vercel.app |
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/search.yaml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/search.yaml (1)
6-9
: Ensure temporary trigger is removed after testing
The TODO comment indicates this push trigger is temporary. Consider:
- Adding a follow-up issue to track the removal
- Including acceptance criteria for when this can be removed
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
🧹 Outside diff range and nitpick comments (1)
README.md (1)
122-122
: LGTM! Consider additional security enhancements.Good improvement in clarifying the type of API key required and emphasizing its sensitive nature. To further enhance security, consider adding:
- A note about not committing the
.env
file to version control- Instructions for securely sharing API keys within the team
- A template
.env.example
file with placeholder valuesAdd these lines after the existing instructions:
Create an .env file with `APPLICATION_ID` and the `API_KEY` (Admin API Key w/ write access, should be kept secret). If you don't have those, one for the Engineering Managers should be able to help you. + +> **Important:** Never commit the `.env` file to version control. Add it to your `.gitignore` file. + +For reference, create a `.env.example` file with the following structure: +``` +APPLICATION_ID=your_app_id_here +API_KEY=your_admin_api_key_here +```
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
README.md
(1 hunks)docusaurus.config.js
(1 hunks)
🧰 Additional context used
🪛 Gitleaks
docusaurus.config.js
30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⚡️ Lighthouse ReportMobile (view report)
Desktop (view report)
Lighthouse ran on https://uniswap-docs-staging-a6xgw48ic-uniswap.vercel.app |
⚡️ Lighthouse ReportMobile (view report)
Desktop (view report)
Lighthouse ran on https://uniswap-docs-staging-6ixvephyp-uniswap.vercel.app |
⚡️ Lighthouse ReportMobile (view report)
Desktop (view report)
Lighthouse ran on https://uniswap-docs-staging-55pnxqxef-uniswap.vercel.app |
Description
Fixes
Chores
Documentation
Type(s) of changes
Motivation for PR
#7
How Has This Been Tested?
The crawl worked locally and it's working in this build that was triggered with a push event that I added to this branch for testing (since removed)
Applicable screenshots
Follow-up PR
When we merge this to https://github.com/Uniswap/docs, we need to ask the Uniswap developers to update the
API Key
secret in the Github Actions env to the Algolia Admin API Key.