Conversation
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a Linux CI step in Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner
participant Installer as TeX Installer
participant Repo as Repository
participant Phino as phino CLI
participant LaTeX as pdflatex
Runner->>Installer: install TeX Live (scheme-basic, eolang, 2025)
Runner->>Repo: checkout code
Runner->>Repo: run cabal build
Repo-->>Runner: locate `phino` binary
Runner->>Repo: write minimal LaTeX (eolang + macros) to article.tex
Runner->>Phino: run `phino explain --normalize` and pipe rewrite -> LaTeX
Phino-->>Repo: emit LaTeX content into article.tex
Runner->>LaTeX: run `pdflatex article.tex`
LaTeX-->>Runner: compilation result (PDF or errors)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds TeXLive setup and LaTeX document generation testing to the CI workflow to ensure that the phino tool generates valid LaTeX output that can be compiled successfully.
Changes:
- Added TeXLive setup action to install required LaTeX packages
- Added a test step that generates a LaTeX document using phino's
explainandrewritecommands - Added pdflatex compilation to validate the generated LaTeX
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/cabal.yml
Outdated
| echo '{[[ x -> [[ y -> ? ]].y ]]}' | "${e}" rewrite --normalize --output=latex --sequence | ||
| echo "\end{document}" | ||
| ) > article.tex | ||
| pdflatex article |
There was a problem hiding this comment.
The pdflatex command is missing error handling. If the LaTeX compilation fails, the step should fail. Add set -e at the beginning of the script or use pdflatex -halt-on-error article to ensure the step fails on compilation errors.
| pdflatex article | |
| pdflatex -halt-on-error article |
.github/workflows/cabal.yml
Outdated
| - uses: teatimeguest/setup-texlive-action@v3.3.4 | ||
| with: | ||
| update-all-packages: true | ||
| packages: scheme-basic geometry xcolor naive-ebnf microtype eolang |
There was a problem hiding this comment.
The TeXLive setup action is being run unconditionally on all platforms including Windows. If the LaTeX document generation step is only intended to run on Unix-like systems (due to comment ID 002), consider adding a platform condition to this step as well to avoid unnecessary setup on Windows.
.github/workflows/cabal.yml
Outdated
| - run: cabal install hpc-codecov-0.6.3.0 --overwrite-policy=always | ||
| - run: make clean coverage | ||
| - run: | | ||
| e=$(find .stack-work/install/ -name phino | head -1) |
There was a problem hiding this comment.
The script is searching for the phino executable in .stack-work/install/, but this is a cabal workflow that builds with cabal, not stack. Cabal builds executables in the dist-newstyle directory. Use cabal list-bin phino to get the correct path to the executable, as done in the release-binary.yml workflow.
| e=$(find .stack-work/install/ -name phino | head -1) | |
| e=$(cabal list-bin phino | head -1) |
.github/workflows/cabal.yml
Outdated
| - run: | | ||
| e=$(find .stack-work/install/ -name phino | head -1) | ||
| ( | ||
| echo "\documentclass{article}" | ||
| echo "\usepackage{eolang}" | ||
| echo "\begin{document}" | ||
| echo "\newcommand\scopeof[1]{#1}" | ||
| echo "\newcommand\isnormal[1]{#1}" | ||
| "${e}" explain --normalize | ||
| echo '{[[ x -> [[ y -> ? ]].y ]]}' | "${e}" rewrite --normalize --output=latex --sequence | ||
| echo "\end{document}" | ||
| ) > article.tex | ||
| pdflatex article |
There was a problem hiding this comment.
The workflow runs on multiple operating systems (macOS, Ubuntu, Windows) according to the matrix strategy, but this script uses bash-specific syntax without specifying a shell. On Windows, this will fail because the default shell is PowerShell. Add a conditional to skip this step on Windows or specify shell: bash for the step.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/cabal.yml:
- Around line 60-72: The run step that builds article.tex and calls pdflatex
uses bash-specific constructs (e.g., $(find .stack-work/install/ -name phino |
head -1), command substitution and pipes) so add a top-level "shell: bash" entry
to that step so the commands execute on Windows runners; update the job step
containing the multi-line run block (the block that defines e, constructs
article.tex with echo and calls pdflatex) to include "shell: bash" immediately
alongside the existing "run:" key.
- Around line 63-70: Replace the multiple echo calls that emit the LaTeX
preamble/postamble and the dynamic printed LaTeX line with a heredoc for the
static block and printf for the dynamic string to address shellcheck SC2028 and
improve portability: wrap the static lines (the lines producing
"\documentclass{article}" through "\newcommand\isnormal[1]{`#1`}" and the final
"\end{document}") in a single cat <<'EOF' heredoc so backslashes are preserved,
and replace the echo that pipes '{[[ x -> [[ y -> ? ]].y ]]}' into "${e}"
rewrite with printf '%s\n' '{[[ x -> [[ y -> ? ]].y ]]}' so the "${e}"
explain/rewrite invocations remain unchanged and behavior of the e variable and
its flags (--normalize, --output=latex, --sequence) is preserved.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/cabal.yml:
- Around line 68-77: The printf invocations that build article.tex are missing
trailing newlines, causing the LaTeX to be emitted on one line; update each
printf (the '\\documentclass{article}', '\\usepackage{eolang}',
'\\begin{document}', '\\newcommand\\scopeof[1]{`#1`}',
'\\newcommand\\isnormal[1]{`#1`}', and '\\end{document}' calls) to include a
terminating '\n' (or replace the whole block with a heredoc that preserves
newlines) so article.tex contains proper line breaks and readable LaTeX for
easier debugging.
♻️ Duplicate comments (1)
.github/workflows/cabal.yml (1)
78-78: Consider adding-halt-on-errorfor more reliable failure detection.While
set -eon line 63 helps,pdflatexdoesn't always return non-zero for all LaTeX errors. Using-halt-on-errorensures immediate failure on the first error.- pdflatex article + pdflatex -halt-on-error article
🧹 Nitpick comments (1)
.github/workflows/cabal.yml (1)
65-66: Consider usingcabal list-binfor a more idiomatic executable lookup.The
findapproach works but is fragile if multiple builds exist.cabal list-bin phinoreturns the correct path directly and is consistent with other workflows in this repository.- e=$(find ./dist-newstyle/ -name phino -type f | head -1) - if [ -z "${e}" ]; then exit 1; fi + e=$(cabal list-bin phino)
|
@maxonfjvipon take a look |
|
@rultor merge |
@maxonfjvipon OK, I'll try to merge now. You can check the progress of the merge here. |
@maxonfjvipon Done! FYI, the full log is here (took me 17min). |
3 participants
closes #600
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.