Migration GitHub to circleci #1185
Conversation
…or the publish-algolia-search job
…package management in publish-algolia-search job
…anage dependencies with pnpm caching
…tor and streamline job steps
….js 20.x with pnpm
…cleCI configuration
…ndex in separate command
…n specific branches
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…lia, breadcrumbs, and Lychee jobs
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces an updated CircleCI configuration file ( A command Additionally, multiple GitHub Actions workflows are deleted, including those for Algolia indexing, breadcrumb checks, link checks, linting, and monthly issue and PR metrics reporting. The overall structure of the CI/CD pipeline is expanded to include additional functionality for error handling, reporting, and scheduled jobs. Sequence DiagramsequenceDiagram
participant CircleCI
participant GitHub
participant Algolia
participant DeveloperMetrics
CircleCI->>GitHub: Trigger merge-workflow
GitHub->>CircleCI: Run algolia job
CircleCI->>Algolia: Create and upload index
CircleCI->>GitHub: Trigger pr-workflow
GitHub->>CircleCI: Run breadcrumbs, links, lint jobs
CircleCI->>DeveloperMetrics: Trigger monthly-workflow
DeveloperMetrics->>CircleCI: Generate metrics report
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.circleci/config.yml (2)
1-4: Consider updating to the latest Node.js orb versionThe configuration uses
circleci/node@7.0.0, but version 7.0.1 is available with bug fixes and improvements.- node: circleci/node@7.0.0 + node: circleci/node@7.0.1
5-17: Maintain consistency in executor configurationsThe
working_directoryis only set for the Rust executor. Consider setting it consistently across all executors for better maintainability.node20: docker: - image: cimg/node:20.11.1 resource_class: medium + working_directory: ~/project ubuntu: machine: image: ubuntu-2204:current + working_directory: ~/project
|
@raffaele-oplabs you've requested review, can you update the description to clarify if we're clear to merge if everything looks good? |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.circleci/config.yml(1 hunks)
🔇 Additional comments (3)
.circleci/config.yml (3)
19-39: Optimize Node.js setup and improve error handling
Several improvements can be made to the setup-node command:
- Node.js installation is redundant when using the
node20executor - pnpm is installed globally twice
- The curl command lacks error handling
50-53:
Secure sensitive Algolia credentials
The Algolia credentials are exposed as environment variables in the job configuration. While they are referenced as variables, ensure they are properly secured in the CircleCI context.
Recommendations:
- Verify that the
algolia-searchcontext is properly configured - Consider using CircleCI's encrypted environment variables
- Add documentation about required context setup
✅ Verification successful
Environment variables are properly documented and handled
The Algolia credentials in the CircleCI configuration are properly referenced and documented. The notes/algolia-search.md file clearly outlines:
- The required environment variables and their purpose
- Documentation for the indexing process
- Automated handling through CI/CD
The environment variables are:
- Used as expected in the CI configuration
- Properly documented in the repository
- Following security best practices by using CircleCI's environment variable references
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the context exists and contains the required environment variables
circleci context list ethereum-optimism | grep algolia-search
circleci context show ethereum-optimism/algolia-search
Length of output: 318
Script:
#!/bin/bash
# Let's check if these environment variables are referenced in other configuration files
# or if there's any documentation about their setup
rg -l "ALGOLIA" --type yaml
rg -l "algolia" --type md
Length of output: 93
Script:
#!/bin/bash
# Let's check the content of these files to understand how the Algolia credentials are documented
cat notes/algolia-search.md
cat notes/README.md
Length of output: 1662
4-4: Consider using a more stable version of the custom orb
The ethereum-optimism/circleci-utils orb is at version 0.0.6, which suggests it's in early development. Early versions may contain breaking changes or bugs.
Consider:
- Pin to a more stable version if available
- Document any known limitations or issues with this early version
…n CircleCI configuration
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.circleci/config.yml (2)
3-5: Pin orb versions more strictly for better stabilityConsider updating the orb versions:
- Pin node orb to a specific patch version for better stability
- The utils orb is in early development (0.x.x) which may introduce breaking changes
- node: circleci/node@7.0.0 - utils: ethereum-optimism/circleci-utils@0.0.6 + node: circleci/node@7.0.1 + utils: ethereum-optimism/circleci-utils@0.0.6 # TODO: Update when 1.0.0 is released
21-53: Add environment variable validation for Slack notificationsThe Slack notification command should validate the required environment variables before attempting to send notifications.
notify_slack_on_failure: steps: + - run: + name: Validate Slack environment variables + command: | + if [ -z "${SLACK_DEFAULT_CHANNEL}" ]; then + echo "Error: SLACK_DEFAULT_CHANNEL is not set" + exit 1 + fi - slack/notify:
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.circleci/config.yml(1 hunks)
🔇 Additional comments (3)
.circleci/config.yml (3)
14-14: Pin Ubuntu image version for consistent builds
Using the current tag for the Ubuntu image could lead to inconsistent builds as the image may change unexpectedly.
54-72: Optimize Node.js setup and improve error handling
Several improvements can be made to the setup-node command.
246-299:
Add missing workflow features from GitHub Actions
Several critical features are missing from the CircleCI configuration:
- Concurrency controls for workflows
- Some GitHub Actions workflows haven't been migrated
Add concurrency controls to prevent multiple builds of the same branch:
workflows:
pr-workflow:
+ when:
+ and:
+ - not:
+ equal: [scheduled_pipeline, << pipeline.trigger_source >>]
jobs:
- breadcrumbs:
+ name: << pipeline.git.branch >>
filters:
branches:
only:
- main
- /pull\/\d+/
raffaele-oplabs
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.circleci/config.yml (1)
16-18: Specify resource class for Rust executorThe
node20executor has a specified resource class, but therustexecutor doesn't. Consider adding a resource class to ensure consistent resource allocation.rust: docker: - image: cimg/rust:1.75.0 # CircleCI's Rust Docker image + resource_class: medium working_directory: ~/project
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.circleci/config.yml(1 hunks)
🔇 Additional comments (6)
.circleci/config.yml (6)
14-14: Pin Ubuntu image version for consistent builds
Using the current tag for the Ubuntu image could lead to inconsistent builds as the image may change unexpectedly.
- image: ubuntu-2204:current
+ image: ubuntu-2204:2024.01.154-73: Optimize Node.js setup and improve error handling
Several improvements can be made to the setup-node command:
- Node.js installation is redundant when using the
node20executor - pnpm is installed globally twice
- The curl command lacks error handling
setup-node:
steps:
- run:
name: Install Node.js (20.x) and pnpm
command: |
- curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash -
- sudo apt-get install -y nodejs
- npm install -g pnpm
+ if [ ! -x "$(command -v node)" ]; then
+ curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash - || exit 1
+ sudo apt-get install -y nodejs
+ fi
+ if [ ! -x "$(command -v pnpm)" ]; then
+ npm install -g pnpm
+ fi
pnpm --version
- restore_cache:
keys:
- v1-pnpm-cache-{{ checksum "pnpm-lock.yaml" }}
- run:
name: Install dependencies
- command: npm install -g pnpm && pnpm install
+ command: pnpm install107-141: Enhance security and reliability of links job
Several improvements can be made to the job:
- The Lychee repository should be pinned to a specific version
- Long-running operations should have timeouts
- Consider using pre-built Lychee binary instead of building from source
- run:
name: Checkout lycheeverse/lychee
command: |
- git clone https://github.com/lycheeverse/lychee.git lychee
+ git clone --depth 1 --branch v0.14.3 https://github.com/lycheeverse/lychee.git lychee
+ no_output_timeout: 10m
- run:
name: Build Lychee
command: |
cd lychee
cargo build --release
+ no_output_timeout: 20m210-244: Create a reusable command for tutorial jobs
The tutorial jobs share similar structure. Consider creating a reusable command to reduce duplication.
+commands:
+ run-tutorial:
+ parameters:
+ script:
+ type: string
+ steps:
+ - checkout
+ - setup-node
+ - run:
+ name: Run tutorial
+ command: node << parameters.script >>
+ environment:
+ TUTORIAL_PRIVATE_KEY: ${TUTORIAL_PRIVATE_KEY}
+ - notify_slack_on_failure
jobs:
cross-dom-bridge-eth:
executor: ubuntu
steps:
- - checkout
- - setup-node
- - run:
- name: Run tutorial
- command: node ./public/tutorials/cross-dom-bridge-eth.js
- environment:
- TUTORIAL_PRIVATE_KEY: ${TUTORIAL_PRIVATE_KEY}
- - notify_slack_on_failure
+ - run-tutorial:
+ script: ./public/tutorials/cross-dom-bridge-eth.js246-279: Clean up and improve workflow configuration
Several improvements are recommended:
- Remove commented out code
- Consider adding job dependencies using
requires - Improve pull request branch pattern
workflows:
merge-workflow:
jobs:
- algolia:
context: algolia-search
filters:
branches:
only: main
- # - algolia:
- # name: Algolia Index Update
- # context: algolia-search
- # filters:
- # branches:
- # only: main
pr-workflow:
jobs:
- breadcrumbs:
name: Breadcrumb Check
filters:
branches:
only:
- main
- - /pull\/\d+/
+ - /pull\/.*/
- - links
+ - links:
+ requires:
+ - "Breadcrumb Check"246-302: CircleCI configuration requires additional workflow features
The CircleCI configuration is missing several critical features present in the GitHub Actions workflows:
- Tutorial verification workflow with sequential jobs:
- Missing concurrency controls (
concurrency.groupandcancel-in-progresssettings)
- Missing concurrency controls (
- Slack notifications:
- GitHub Actions has Slack notifications configured for tutorial failures
- Monthly metrics workflows:
- Missing permissions configuration for issues and pull requests
Please update the CircleCI configuration to include these missing features before completing the migration.
#!/bin/bash
# Check for existing GitHub Actions workflows
echo "Checking for GitHub Actions workflows..."
fd -e yml -e yaml . .github/workflows/
echo "Analyzing workflow files..."
for file in $(fd -e yml -e yaml . .github/workflows/); do
echo "=== $file ==="
yq '.jobs | keys' "$file" 2>/dev/null
done…ectly for Algolia settings
…s for Algolia settings
… links, and linting
…veloper-pr-metrics job from CircleCI configuration
…oper-pr-metrics job in CircleCI configuration
…veloper-pr-metrics job from CircleCI configuration
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.circleci/config.yml (1)
15-18: Specify resource class for Rust executorThe rust executor should specify a resource class to ensure consistent resource allocation, similar to the node20 executor.
rust: docker: - image: cimg/rust:1.75.0 # CircleCI's Rust Docker image + resource_class: medium working_directory: ~/project
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.circleci/config.yml(1 hunks)
🔇 Additional comments (5)
.circleci/config.yml (5)
14-14: Pin Ubuntu image version for consistent builds
Using the current tag for the Ubuntu image could lead to inconsistent builds as the image may change unexpectedly.
- image: ubuntu-2204:current
+ image: ubuntu-2204:2024.01.154-73: Optimize Node.js setup and improve error handling
Several improvements can be made to the setup-node command:
- Node.js installation is redundant when using the
node20executor - pnpm is installed globally twice
- The curl command lacks error handling
setup-node:
steps:
- run:
name: Install Node.js (20.x) and pnpm
command: |
- curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash -
- sudo apt-get install -y nodejs
- npm install -g pnpm
+ if [ ! -x "$(command -v node)" ]; then
+ curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash - || exit 1
+ sudo apt-get install -y nodejs
+ fi
+ if [ ! -x "$(command -v pnpm)" ]; then
+ npm install -g pnpm
+ fi
pnpm --version
- restore_cache:
keys:
- v1-pnpm-cache-{{ checksum "pnpm-lock.yaml" }}
- run:
name: Install dependencies
- command: npm install -g pnpm && pnpm install
+ command: pnpm install109-143: Enhance security and reliability of links job
Several improvements can be made to the job:
- The Lychee repository should be pinned to a specific version
- Long-running operations should have timeouts
- Consider using pre-built Lychee binary instead of building from source
- run:
name: Checkout lycheeverse/lychee
command: |
- git clone https://github.com/lycheeverse/lychee.git lychee
+ git clone --depth 1 --branch v0.14.3 https://github.com/lycheeverse/lychee.git lychee
+ no_output_timeout: 10m
- run:
name: Build Lychee
command: |
cd lychee
cargo build --release
+ no_output_timeout: 20m187-221: Create a reusable command for tutorial jobs
The tutorial jobs share similar structure. Consider creating a reusable command to reduce duplication.
+commands:
+ run-tutorial:
+ parameters:
+ script:
+ type: string
+ steps:
+ - checkout
+ - setup-node
+ - run:
+ name: Run tutorial
+ command: node << parameters.script >>
+ environment:
+ TUTORIAL_PRIVATE_KEY: ${TUTORIAL_PRIVATE_KEY}
+ - notify_slack_on_failure223-256: Migration to CircleCI is incomplete - several workflows are missing
The current CircleCI configuration only covers:
- Algolia indexing
- Breadcrumbs checking
- Link checking
Missing workflows that need to be migrated:
- Markdown linting workflow
- Monthly issue metrics reporting
- Monthly PR metrics reporting
- Tutorial verification workflows (with 3 sequential jobs)
Key missing features:
- Slack notifications for tutorial failures
- Proper concurrency controls for tutorials
- Scheduled runs for metrics and tutorials
- Proper environment variable setup for tutorials
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.circleci/config.yml (3)
18-38: 🛠️ Refactor suggestionOptimize Node.js setup and improve error handling
The setup-node command has redundant steps and lacks error handling.
setup-node: steps: - run: name: Install Node.js (20.x) and pnpm command: | - curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash - - sudo apt-get install -y nodejs - npm install -g pnpm + if [ ! -x "$(command -v node)" ]; then + curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash - || exit 1 + sudo apt-get install -y nodejs + fi + if [ ! -x "$(command -v pnpm)" ]; then + npm install -g pnpm + fi pnpm --version - restore_cache: keys: - v1-pnpm-cache-{{ checksum "pnpm-lock.yaml" }} - run: name: Install dependencies - command: npm install -g pnpm && pnpm install + command: pnpm install
39-151:⚠️ Potential issueMigration to CircleCI is incomplete - several workflows are missing
The configuration is missing critical jobs from the GitHub Actions workflows.
Missing jobs:
- Tutorial verification jobs (cross-dom-bridge-eth, send-tx-from-eth, sdk-estimate-costs)
- Proper concurrency controls
- Slack notifications for failures
74-108: 🛠️ Refactor suggestionEnhance security and reliability of the links job
The Lychee link checker setup needs security improvements.
- run: name: Checkout lycheeverse/lychee command: | - git clone https://github.com/lycheeverse/lychee.git lychee + git clone --depth 1 --branch v0.14.3 https://github.com/lycheeverse/lychee.git lychee + no_output_timeout: 10m - run: name: Build Lychee command: | cd lychee cargo build --release + no_output_timeout: 20m
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.circleci/config.yml(1 hunks).github/workflows/monthly-issue-metircs.yml(0 hunks).github/workflows/monthly-pr-metrics.yml(0 hunks).github/workflows/tutorials.yml(0 hunks)
💤 Files with no reviewable changes (3)
- .github/workflows/tutorials.yml
- .github/workflows/monthly-pr-metrics.yml
- .github/workflows/monthly-issue-metircs.yml
🔇 Additional comments (2)
.circleci/config.yml (2)
11-13: 🛠️ Refactor suggestion
Pin Ubuntu image version for consistent builds
Using the current tag for the Ubuntu image could lead to inconsistent builds as the image may change unexpectedly.
- image: ubuntu-2204:current
+ image: ubuntu-2204:2024.01.1Likely invalid or redundant comment.
1-4: 🛠️ Refactor suggestion
Pin the utils orb to a stable version
Using a development version (0.0.7) of the utils orb can lead to unexpected behavior if the orb is updated.
- utils: ethereum-optimism/circleci-utils@0.0.7
+ utils: ethereum-optimism/circleci-utils@1.0.0 # Replace with actual stable versionLikely invalid or redundant comment.
raffaele-oplabs
changed the title
This pull request consolidates multiple GitHub Actions workflows into a single CircleCI configuration file, streamlining the CI/CD process. The changes include adding new executors and commands in CircleCI, and removing the corresponding GitHub Actions workflows.
CircleCI Configuration Enhancements:
.circleci/config.yml: Added CircleCI configuration with executors for Node.js, Ubuntu, and Rust, and defined jobs for Algolia indexing, breadcrumbs check, Markdown linting, link checking, and developer metrics reporting.Removal of GitHub Actions Workflows:
.github/workflows/algolia.yml: Removed the GitHub Actions workflow for Algolia index updates, now handled by CircleCI..github/workflows/breadcrumbs.yml: Removed the GitHub Actions workflow for breadcrumb checks, now handled by CircleCI..github/workflows/links.yml: Removed the GitHub Actions workflow for link checking, now handled by CircleCI..github/workflows/lint.yml: Removed the GitHub Actions workflow for Markdown linting, now handled by CircleCI..github/workflows/monthly-issue-metircs.yml: Removed the GitHub Actions workflow for monthly issue metrics reporting, now handled by CircleCI..github/workflows/monthly-pr-metrics.yml: Removed the GitHub Actions workflow for monthly PR metrics reporting, now handled by CircleCI..github/workflows/tutorials.yml: Removed the GitHub Actions workflow for checking tutorials. This was removed and not replaces.