-
Notifications
You must be signed in to change notification settings - Fork 26
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
Ci workflow #673
Ci workflow #673
Conversation
WalkthroughThe changes involve modifications to two GitHub Actions workflow files: Changes
Possibly related PRs
Suggested reviewers
Poem
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
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: 4
🧹 Outside diff range and nitpick comments (5)
.github/workflows/build.yml (3)
6-22
: Consider enhancing input parameter descriptionsWhile the input parameters are well-structured, their descriptions could be more detailed to guide users:
network: - description: "Network to deploy to" + description: "Target network for deployment (e.g., testnet, mainnet)" required: true type: string kernel_address: - description: "Kernel address" + description: "Address of the kernel contract (if updating existing deployment)" required: false type: string deploy_os: - description: "Deploy OS" + description: "Whether to deploy the operating system components (true/false)" required: false type: boolean contracts: - description: "Contracts to deploy" + description: "Comma-separated list of contract names to deploy (empty for all)" required: false type: string
81-96
: Enhance error handling and loggingWhile basic error handling is present, it could be improved to provide more context and debugging information.
try { + console.log('Triggering deploy workflow...'); await github.rest.actions.createWorkflowDispatch({ owner: context.repo.owner, repo: context.repo.repo, workflow_id: 'deploy.yml', ref: github.ref, inputs: { network: '${{ inputs.network }}', kernel_address: '${{ inputs.kernel_address }}', deploy_os: '${{ inputs.deploy_os }}', contracts: '${{ inputs.contracts }}' } }); + console.log('Deploy workflow triggered successfully'); } catch (error) { - core.setFailed(`Failed to trigger deploy workflow: ${error.message}`); + console.error('Error details:', error); + core.setFailed(`Failed to trigger deploy workflow: ${error.message} + \nStatus: ${error.status} + \nResponse: ${JSON.stringify(error.response?.data || {})}`); } +🧰 Tools
🪛 yamllint (1.35.1)
[error] 96-96: no new line character at the end of file
(new-line-at-end-of-file)
96-96
: Add newline at end of fileFollowing YAML best practices, ensure there's a newline at the end of the file.
core.setFailed(`Failed to trigger deploy workflow: ${error.message}`); } +
🧰 Tools
🪛 yamllint (1.35.1)
[error] 96-96: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/deploy.yml (2)
110-110
: Fix job indentationThe job indentation should be 4 spaces to match the workflow file structure.
- needs: [deploy, build_schemas] + needs: [deploy, build_schemas]🧰 Tools
🪛 yamllint (1.35.1)
[warning] 110-110: wrong indentation: expected 4 but found 6
(indentation)
161-168
: Fix code quality issuesThere are some code quality issues that should be addressed:
- Remove trailing spaces on line 161
- Add a newline at the end of the file
- + if [[ -n $(git status --porcelain) ]]; then git add . git commit -m "chore: Update schema for kernel: ${{ inputs.kernel_address }}" git push else echo "No changes to commit" - fi + fi +🧰 Tools
🪛 yamllint (1.35.1)
[error] 161-161: trailing spaces
(trailing-spaces)
[error] 168-168: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/build.yml
(2 hunks).github/workflows/deploy.yml
(4 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/build.yml
[error] 96-96: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/deploy.yml
[warning] 110-110: wrong indentation: expected 4 but found 6
(indentation)
[error] 161-161: trailing spaces
(trailing-spaces)
[error] 168-168: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.4)
.github/workflows/deploy.yml
115-115: shellcheck reported issue in this script: SC2086:info:2:28: Double quote to prevent globbing and word splitting
(shellcheck)
115-115: shellcheck reported issue in this script: SC2086:info:4:36: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (3)
.github/workflows/deploy.yml (3)
18-18
: LGTM!
The simplified description for the 'contracts' input parameter is clear and concise.
79-79
: LGTM!
The dependency order ensures all required artifacts are built before deployment.
34-34
: Verify that make version-map
works without sudo
The removal of sudo
from the command is good practice for security. However, we should verify that the command still works as expected.
✅ Verification successful
Removing sudo
from make version-map
is safe
The script build_version_map.sh
only performs standard file operations:
- Creates an artifacts directory
- Writes a JSON file with contract versions
- Uses basic commands like
mkdir
,touch
,echo
None of these operations require elevated privileges as long as the user has write permissions in the repository directory, which is standard in CI environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the Makefile contains any commands that might require sudo
rg -A 5 "version-map:" Makefile
Length of output: 218
Script:
#!/bin/bash
# Check the content of build_version_map.sh to verify if it requires sudo privileges
cat scripts/build_version_map.sh
Length of output: 1147
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
🧹 Outside diff range and nitpick comments (4)
.github/workflows/build.yml (2)
6-22
: Enhance input parameters documentation and validationThe workflow inputs could benefit from more detailed descriptions and validation:
kernel_address
: Specify the expected format/patterndeploy_os
: Clarify what true/false values representcontracts
: Document the expected format (e.g., comma-separated list, JSON array)network
: Consider listing valid network optionsExample improvement:
network: - description: "Network to deploy to" + description: "Target network for deployment (e.g., testnet, mainnet)" required: true type: string kernel_address: - description: "Kernel address" + description: "Contract address of the kernel in bech32 format" required: false type: string deploy_os: - description: "Deploy OS" + description: "Whether to deploy the operating system contracts (true/false)" required: false type: boolean contracts: - description: "Contracts to deploy" + description: "Comma-separated list of contract names to deploy (e.g., 'kernel,ado')" required: false type: string
71-96
: LGTM! Well-structured job implementationThe trigger-deploy job is well implemented with:
- Proper job dependencies
- Secure token usage
- Error handling
- Correct input forwarding
Add a newline at the end of the file to comply with POSIX standards.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 96-96: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/deploy.yml (2)
28-31
: Consider pinning the Rust toolchain versionWhile using the stable channel is good, pinning to a specific version would ensure better reproducibility across different environments and time periods.
- - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/[email protected] with: - toolchain: stable + toolchain: 1.76.0
114-114
: Fix YAML formatting issuesThere are some formatting inconsistencies in the file:
- Incorrect indentation at line 114 (should be 4 spaces)
- Trailing spaces at lines 123 and 172
- needs: [deploy, build_schemas] + needs: [deploy, build_schemas] - + - +Also applies to: 123-123, 172-172
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 114-114: wrong indentation: expected 4 but found 6
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/build.yml
(2 hunks).github/workflows/deploy.yml
(4 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/build.yml
[error] 96-96: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/deploy.yml
[warning] 114-114: wrong indentation: expected 4 but found 6
(indentation)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 179-179: no new line character at the end of file
(new-line-at-end-of-file)
🪛 actionlint (1.7.4)
.github/workflows/deploy.yml
119-119: shellcheck reported issue in this script: SC2086:info:6:28: Double quote to prevent globbing and word splitting
(shellcheck)
119-119: shellcheck reported issue in this script: SC2086:info:8:36: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (5)
.github/workflows/build.yml (1)
79-79
: Verify CI_PAT permissions
Ensure that the CI_PAT
secret has the necessary permissions to trigger workflows:
- It should be a Personal Access Token or GitHub App token
- It needs
workflow
scope to trigger other workflows
✅ Verification successful
CI_PAT permissions are working as expected
The workflow has been successfully triggered multiple times via workflow_dispatch events, indicating that the CI_PAT token has the correct permissions. The successful runs and the proper usage of the token in the workflow file confirm that the secret is properly configured.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the workflow has been previously triggered successfully
gh run list --workflow=deploy.yml --json conclusion,triggeredBy --jq '.[] | select(.triggeredBy.event == "workflow_dispatch") | .conclusion' | head -n 1
Length of output: 391
Script:
#!/bin/bash
# Check workflow runs with correct fields
gh run list --workflow=build.yml --json event,conclusion --jq '.[] | select(.event == "workflow_dispatch") | .conclusion' | head -n 5
# Check for workflow_dispatch triggers in the workflow file
rg "workflow_dispatch|github-token|CI_PAT" .github/workflows/build.yml -A 2 -B 2
Length of output: 638
.github/workflows/deploy.yml (4)
18-18
: LGTM!
The simplified description for the contracts input parameter is clear and concise.
44-65
: LGTM! Well-structured schema build job
The job implementation includes:
- Proper error handling with set -eo pipefail
- Appropriate artifact retention period
- Consistent permission gating with other jobs
83-83
: LGTM! Correct dependency chain
The addition of build_schemas as a dependency ensures proper workflow sequencing.
113-179
: LGTM! Well-implemented schema parser trigger
The implementation includes:
- Proper kernel address validation
- Secure GitHub token usage
- Appropriate error handling
- Clean git operations with proper user configuration
🧰 Tools
🪛 actionlint (1.7.4)
119-119: shellcheck reported issue in this script: SC2086:info:6:28: Double quote to prevent globbing and word splitting
(shellcheck)
119-119: shellcheck reported issue in this script: SC2086:info:8:36: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
[warning] 114-114: wrong indentation: expected 4 but found 6
(indentation)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 179-179: no new line character at the end of file
(new-line-at-end-of-file)
Motivation
Automate the process
Summary by CodeRabbit