Fix CI and upgrade supported Vim version #34
Conversation
|
Warning Rate limit exceeded@lambdalisue has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduce two new GitHub Actions workflow files, Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
.github/workflows/neovim.yml (2)
29-33: Consider pinning the action to a specific commit.While
rhysd/action-setup-vim@v1is stable, consider pinning to a specific commit SHA for enhanced security and reproducibility.- - uses: rhysd/action-setup-vim@v1 + - uses: rhysd/action-setup-vim@755c18f80e522dd4e4675c7f1ae1bfb3e0a67c1c # v1
34-43: Consider enhancing the test runner configuration.While the basic test setup works, consider adding:
- Verbose output flag for better CI logs
- Test timeout to prevent hanging
- Error reporting format for better GitHub integration
- ./vim-themis/bin/themis + ./vim-themis/bin/themis --reporter spec --timeout 30 --runtimepath ..github/workflows/vim.yml (2)
24-28: Optimize the repository checkouts.Consider combining the two checkout actions into one by using the
actions/checkout@v4action's support for multiple repositories:- - uses: actions/checkout@v4 - - uses: actions/checkout@v4 - with: - repository: thinca/vim-themis - path: vim-themis + - uses: actions/checkout@v4 + with: + submodules: true + fetch-depth: 0 + - uses: actions/checkout@v4 + with: + repository: thinca/vim-themis + path: .vim-themis
36-40: Document the Windows temp directory workaround.The XXX comment indicates a temporary workaround. Consider:
- Adding a link to the GitHub issue in the comment
- Creating a tracking issue to remove this workaround when fixed
Would you like me to create a tracking issue for removing this workaround once GitHub Actions fixes the Windows temp directory issue?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
.github/workflows/neovim.yml(1 hunks).github/workflows/test.yml(0 hunks).github/workflows/vim.yml(1 hunks)README.md(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/test.yml
🔇 Additional comments (7)
.github/workflows/neovim.yml (3)
3-8: LGTM! Well-configured workflow triggers.
The workflow triggers are properly set up with standard events (push to main, pull requests) and include manual dispatch for debugging purposes.
24-29: LGTM! Proper repository checkouts.
Both the main repository and testing framework checkouts are correctly configured using the latest action version.
12-21: Consider updating the minimum supported Neovim version.
While testing v0.4.4 ensures backward compatibility, this version is quite old (released in 2020). Consider:
- Updating to a more recent minimum version (e.g., v0.7.0 or v0.8.0)
- Adding the latest nightly build to catch potential issues early
.github/workflows/vim.yml (2)
1-9: LGTM! Well-configured workflow triggers.
The workflow triggers are appropriately set up for CI, covering all necessary scenarios (main branch pushes, PRs, and manual runs).
10-22: Verify the minimum Vim version requirement.
The matrix configuration is well-structured. However, let's verify if v8.2.5136 is still the minimum required version by checking the referenced issue.
#!/bin/bash
# Description: Verify the minimum Vim version requirement from issue #506
gh issue view 506 --repo lambdalisue/vim-fernREADME.md (2)
1-4: Documentation update is clear and well-structured.
The version requirement changes are clearly presented through the badges, and the rest of the documentation maintains its clarity and organization.
3-4: Verify version requirements across workflows.
The version requirements look good, but let's verify they match the new workflow configurations.
✅ Verification successful
Version requirements in README match workflow configurations
The version requirements shown in the README badges are consistent with the workflow configurations:
- Vim workflow tests v8.2.5136 (matches README badge)
- Neovim workflow tests v0.4.4 (matches README badge)
Both workflows also test against higher versions (Vim nightly and Neovim stable), which is appropriate for the "or above" specification in the badges.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the version requirements in README match the workflow files
# Test: Search for version specifications in workflow files
echo "Checking workflow version specifications..."
rg "version|ver:" .github/workflows/
Length of output: 365
Script:
#!/bin/bash
# Let's check the actual version matrices in the workflow files
echo "=== Checking Vim workflow versions ==="
rg "matrix:" -A 5 .github/workflows/vim.yml
echo -e "\n=== Checking Neovim workflow versions ==="
rg "matrix:" -A 5 .github/workflows/neovim.yml
Length of output: 518
Script:
#!/bin/bash
# Let's get more context by checking the complete workflow files
echo "=== Complete Vim workflow file ==="
cat .github/workflows/vim.yml
echo -e "\n=== Complete Neovim workflow file ==="
cat .github/workflows/neovim.yml
Length of output: 2428
| run: | | ||
| ./vim-themis/bin/themis |
There was a problem hiding this comment.
💡 Codebase verification
Pin vim-themis to the latest stable version v1.7.0
The vim-themis repository has several stable releases available. Currently, the workflow fetches the latest commit without version pinning. To ensure consistent test results, update the checkout action to use tag v1.7.0:
- uses: actions/checkout@v4
with:
repository: thinca/vim-themis
path: vim-themis
ref: v1.7.0🔗 Analysis chain
Consider pinning vim-themis version.
The vim-themis testing framework is being used without version pinning. This could lead to inconsistent test results if breaking changes are introduced.
Let me try another approach to check the repository directly since the GitHub CLI command didn't provide the expected information.
Let me check if there's a specific commit hash or tag being used in other repositories that use vim-themis, which could help us recommend a stable version.
Let me try one final check to see if vim-themis has any tags in its repository directly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if vim-themis has tagged releases we can pin to
gh release list --repo thinca/vim-themis
Length of output: 489
Script:
#!/bin/bash
# Check how vim-themis is being cloned/installed in the workflow
cat .github/workflows/vim.yml
# Also check if there are any git submodules that might contain vim-themis
cat .gitmodules 2>/dev/null || echo "No .gitmodules file found"
Length of output: 1220
Script:
#!/bin/bash
# Search for other popular vim plugins using vim-themis to see how they pin it
gh search repos "vim-themis path:.github/workflows language:yaml" --limit 5 --json fullName,url | jq -r '.[] | .fullName' | while read -r repo; do
echo "=== Checking $repo ==="
gh api "/repos/$repo/contents/.github/workflows" --jq '.[] | .download_url' | while read -r url; do
if [ ! -z "$url" ]; then
curl -s "$url" | grep -A 5 "thinca/vim-themis"
fi
done
done
Length of output: 138
Script:
#!/bin/bash
# Check if the vim-themis directory has any tags after it's checked out
cd vim-themis 2>/dev/null && git tag || echo "Directory not found"
# Also check the remote repository's default branch
git ls-remote --tags https://github.com/thinca/vim-themis
Length of output: 984
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
autoload/nerdfont.vim (1)
2-3: LGTM! Consider adding a safeguard for empty components.The cross-platform path handling implementation looks good. Using
join()with a platform-specific separator is the correct approach.Consider adding a safeguard against empty path components:
-let s:json_dir = join([s:path, 'assets', 'json'], s:sep) +let s:json_dir = join(filter([s:path, 'assets', 'json'], '!empty(v:val)'), s:sep)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
autoload/nerdfont.vim(1 hunks)
🔇 Additional comments (1)
autoload/nerdfont.vim (1)
5-9: LGTM! Verify JSON files accessibility across platforms.
The path construction is now properly handled for cross-platform compatibility.
Let's verify the JSON files are accessible and the paths are correctly constructed:
✅ Verification successful
Path construction is properly implemented for cross-platform compatibility ✅
The verification confirms:
- JSON files exist in the expected
assets/jsondirectory - Path separator (
s:sep) is correctly defined based on the platform usinghas('win32') s:json_diris constructed usingjoin()with the platform-specific separator- No hardcoded path separators are used in vim files
The changes in nerdfont#get_json() align with the codebase's cross-platform path handling approach, ensuring consistent behavior across different operating systems.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify JSON files existence and accessibility
# Expected: Find JSON files in assets/json directory
# Test 1: Check if JSON files exist
echo "Checking JSON files in assets/json directory..."
fd -e json . assets/json/
# Test 2: Verify no hardcoded path separators in vim files
echo "Checking for hardcoded path separators in vim files..."
rg --type vim "assets/json/" .
Length of output: 441
Script:
#!/bin/bash
# Let's verify the path separator handling in the vim files
# Check how s:json_dir and s:sep are defined
echo "Checking s:json_dir and s:sep definitions..."
rg "let\s+s:(json_dir|sep)\s*=" --type vim
# Check all usages of path construction in vim files
echo "Checking path construction patterns..."
rg "\.\.?\s*\.\s*['\"]/" --type vim
rg "join\s*\(.*,\s*['\"]/" --type vim
Length of output: 450
lambdalisue
force-pushed
the
fix-ci
branch
7 times, most recently
from
1441cc7
to
731fefb
Compare
SSIA
Summary by CodeRabbit