-
Notifications
You must be signed in to change notification settings - Fork 0
ci: block rustfmt+cargo-deny; api: CSV include_header examples #27
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
Conversation
zensgit
commented
Sep 23, 2025
- Make Clippy blocking in API job\n- Add Rustfmt Check and Cargo Deny as blocking jobs\n- jive-api/Makefile: add include_header examples for POST/GET CSV\n- AGENTS.md: document include_header flag\n\nIf approved, this enforces formatting and license advisories at PR time.
…; api: Makefile CSV export include_header examples; docs: AGENTS include_header notes
Summary of ChangesHello @zensgit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant improvements to the project's development workflow and API documentation. It strengthens the continuous integration process by enforcing code style with Rustfmt, static analysis with Clippy, and dependency security with Cargo Deny as blocking checks. Additionally, it enhances the API's CSV export functionality by documenting the "include_header" option and providing practical Makefile examples for its usage. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Pull Request Overview
This PR enforces code quality standards by making formatting and security checks blocking in CI, while also adding CSV export documentation and examples. The changes implement stricter CI validation and improve developer documentation for CSV functionality.
- Add blocking Rustfmt and Cargo Deny checks to CI pipeline
- Make Clippy warnings blocking instead of optional
- Add Makefile examples for CSV export with include_header parameter
- Document CSV include_header functionality in AGENTS.md
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/ci.yml | Adds rustfmt-check and cargo-deny jobs as blocking CI steps, makes clippy blocking |
| jive-api/Makefile | Adds convenience targets for CSV export with include_header examples |
| AGENTS.md | Documents CSV export include_header parameter usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.github/workflows/ci.yml
Outdated
| components: rustfmt | ||
| - name: Check formatting (workspace) | ||
| run: | | ||
| rustup component add rustfmt || true |
Copilot
AI
Sep 23, 2025
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.
The || true makes this command always succeed, which could mask failures when rustfmt component installation fails. Since rustfmt is already specified in the toolchain components above, this line is redundant and should be removed.
| rustup component add rustfmt || true |
Copilot uses AI. Check for mistakes.
| - uses: actions/checkout@v4 | ||
| - name: Install cargo-deny | ||
| run: | | ||
| curl -sSfL https://github.com/EmbarkStudios/cargo-deny/releases/download/0.14.24/cargo-deny-0.14.24-x86_64-unknown-linux-musl.tar.gz | tar xz |
Copilot
AI
Sep 23, 2025
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.
Downloading and executing binaries from external URLs without verification poses a security risk. Consider using a GitHub Action like EmbarkStudios/cargo-deny-action or verify the binary checksum after download.
| curl -sSfL https://github.com/EmbarkStudios/cargo-deny/releases/download/0.14.24/cargo-deny-0.14.24-x86_64-unknown-linux-musl.tar.gz | tar xz | |
| set -e | |
| VERSION=0.14.24 | |
| FILE="cargo-deny-${VERSION}-x86_64-unknown-linux-musl.tar.gz" | |
| URL="https://github.com/EmbarkStudios/cargo-deny/releases/download/${VERSION}/${FILE}" | |
| SHA256_URL="${URL}.sha256" | |
| # Download tarball and checksum | |
| curl -sSfL "$URL" -o "$FILE" | |
| curl -sSfL "$SHA256_URL" -o "${FILE}.sha256" | |
| # The .sha256 file contains the checksum and filename, but we need to match the filename | |
| grep "$FILE" "${FILE}.sha256" > checksum.txt | |
| sha256sum -c checksum.txt | |
| # Extract and install | |
| tar xzf "$FILE" |
Copilot uses AI. Check for mistakes.
| @echo "Adminer: http://localhost:8080" | ||
| @echo "RedisInsight: http://localhost:8001" | ||
|
|
||
| # 便捷:导出/审计(支持 include_header 传参) |
Copilot
AI
Sep 23, 2025
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.
[nitpick] The comment is in Chinese while the rest of the file uses English. For consistency and accessibility, consider translating to English: # Convenience: export/audit (supports include_header parameter)
| # 便捷:导出/审计(支持 include_header 传参) | |
| # Convenience: export/audit (supports include_header parameter) |
Copilot uses AI. Check for mistakes.
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.
Code Review
This pull request introduces several improvements to the CI process by making Clippy, Rustfmt, and Cargo Deny checks blocking. It also adds helpful Makefile targets to demonstrate and test CSV export functionality, and documents the include_header parameter in AGENTS.md. The changes are mostly good, but I've found a small issue in one of the new Makefile targets that I've provided a suggestion for.
| curl -s -D - -H "Authorization: Bearer $${TOKEN}" \ | ||
| "http://localhost:$${API_PORT:-8012}/api/v1/transactions/export.csv?include_header=$${HEADER:-true}" \ | ||
| -o /tmp/transactions_export.csv | head -n 20 |
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.
The pipe | head -n 20 in the export-csv-stream target is likely not behaving as intended. The curl command uses -o /tmp/transactions_export.csv to write the response body to a file, and -D - to write headers to standard output. As a result, head -n 20 will only process the response headers, not the CSV content.
To preview the first 20 lines of the downloaded CSV file, you should first let curl finish downloading and then run head on the output file. I've also removed the -D - flag which prints headers to standard output, as it would interfere with previewing the file content.
curl -s -H "Authorization: Bearer $${TOKEN}" \
"http://localhost:$${API_PORT:-8012}/api/v1/transactions/export.csv?include_header=$${HEADER:-true}" \
-o /tmp/transactions_export.csv && head -n 20 /tmp/transactions_export.csv
…o-deny and summary wiring
…rs to fix rustfmt
…mt blocking later