-
Notifications
You must be signed in to change notification settings - Fork 1.9k
workflows: Install NSIS via chocolatey #10858
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
WalkthroughGitHub Actions Windows build workflow updated to install both gzip and NSIS via Chocolatey in a single step; only the step name and install arguments changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches🧪 Generate unit tests
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 |
Signed-off-by: Hiroshi Hatake <[email protected]>
cb83309 to
a79f720
Compare
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: 0
🧹 Nitpick comments (1)
.github/workflows/call-build-windows.yaml (1)
126-130: Install NSIS is good; consider pinning and quieter installs, plus a sanity check.
- To avoid future breakage, pin NSIS (and optionally keep gzip floating) and make installs less noisy.
- Add a quick validation to fail fast if makensis isn’t on PATH.
Apply either of these diffs:
Option A: Keep single step, quieter output (no pin):
- - name: Get gzip command and nsis w/ chocolatey + - name: Install gzip and NSIS via Chocolatey uses: crazy-max/ghaction-chocolatey@v3 with: - args: install gzip nsis -y + args: install gzip nsis -y --no-progress --limit-outputOption B: Split and pin only NSIS:
- - name: Get gzip command and nsis w/ chocolatey - uses: crazy-max/ghaction-chocolatey@v3 - with: - args: install gzip nsis -y + - name: Install gzip via Chocolatey + uses: crazy-max/ghaction-chocolatey@v3 + with: + args: install gzip -y --no-progress --limit-output + + - name: Install NSIS via Chocolatey (pinned) + uses: crazy-max/ghaction-chocolatey@v3 + with: + args: install nsis --version 3.10 -y --no-progress --limit-outputThen add this check right after to verify availability:
- name: Verify NSIS is available run: makensis /VERSION shell: pwsh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/call-build-windows.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
|
The manually kicking CI results are here: |
This is because windows-2025 GHA runner is gradually rolling out so we need to install NSIS explicitly if we use it in our CI.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit