-
Notifications
You must be signed in to change notification settings - Fork 120
Fix name of bicep binary #10838
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
Fix name of bicep binary #10838
Conversation
9f63128 to
55566c8
Compare
|
Code change looks good! Just some comments on documentation and naming references in the repo :) We have a few test scripts that might rely on the Line 161 in bdbaf37
There are also references to Line 179 in bdbaf37
Line 211 in bdbaf37
Line 37 in bdbaf37
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10838 +/- ##
==========================================
- Coverage 50.70% 50.70% -0.01%
==========================================
Files 675 675
Lines 42456 42456
==========================================
- Hits 21529 21527 -2
- Misses 18858 18859 +1
- Partials 2069 2070 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0cbbc82 to
7c99f5f
Compare
|
Apologies for the long delay in responding! I did a text search and have updated all references to rad-bicep. |
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 renames the Bicep binary from "rad-bicep" to "bicep" throughout the codebase to align with standard Bicep tooling conventions. The changes affect code, comments, documentation, and installation scripts.
Key Changes
- Updated binary name constant from "rad-bicep" to "bicep" and environment variable from "RAD_BICEP" to "BICEP"
- Updated all error messages, comments, and documentation references to use "bicep" instead of "rad-bicep"
- Updated installation scripts and CI workflows to reference the new binary name
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/bicep/bicep.go | Changed binary name constant and environment variable name |
| pkg/cli/bicep/build.go | Updated error messages and comments to reference "bicep" instead of "rad-bicep" |
| pkg/cli/bicep/types.go | Updated error messages and comments to use new binary name |
| pkg/cli/cmd/bicep/publishextension/publish.go | Updated comment to reflect new binary name |
| docs/contributing/contributing-code/contributing-code-tests/running-functional-tests.md | Updated documentation to reference "bicep" binary |
| docs/contributing/contributing-code/contributing-code-schema-changes/README.md | Updated file path reference from "rad-bicep" to "bicep" |
| deploy/install.sh | Updated installation messages to use "bicep" instead of "rad-bicep" |
| deploy/install.ps1 | Updated error message to reference "bicep" |
| build/test.mk | Updated Bicep binary path in test command |
| .github/workflows/validate-bicep.yaml | Updated file paths and verification commands to use "bicep" |
| bicep-types | Updated submodule commit reference |
| const ( | ||
| radBicepEnvVar = "RAD_BICEP" | ||
| binaryName = "rad-bicep" | ||
| BicepEnvVar = "BICEP" |
Copilot
AI
Dec 13, 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 environment variable name 'BICEP' is too generic and could conflict with other tools or users' environment variables. Consider using a more specific name like 'RAD_BICEP_PATH' or 'RADIUS_BICEP' to avoid potential conflicts.
| BicepEnvVar = "BICEP" | |
| BicepEnvVar = "RAD_BICEP_PATH" |
|
There has been some discussion of this PR this week and the impact of changing the binary name. The concerns were:
My read of this PR is that:
So the question is should the Radius CLI be downloading binaries for the user or not. On principal, I believe the answer is no, users should be in control of what software is downloaded to their workstation. Additionally, in air-gapped environments, users may need to source binaries from alternative sources such as an internal mirror. @ytimocin is modifying the installation process as specified in this feature spec. In that specification, we propose moving the installation of It makes sense to me that we name the bicep binary as |
|
@brooke-hamilton @ytimocin however you would like to proceed is fine by me. |
|
@nellshamrell, after conversations with @zachcasper and the team, the TLDR; is that we want to keep this PR and merge the change. @zachcasper is correctly pointing out that we have upcoming changes in which we make Bicep a more explicit dependency of Radius, in which users have some choice in the way Bicep is installed with the Radius CLI and with the control plane. When we make those changes, we will want the Bicep binary to keep its original file name instead of renaming it as we do today. The likely way the CLI will interact with Bicep is use Bicep from the Within the context of giving users more choice for installing and using Bicep, it makes sense that we would merge this PR as a first step of the upcoming changes. |
|
@brooke-hamilton understood, I will work on fixing the failing test then :) |
- changes the name of the downloaded bicep binary from "rad-bicep" to "bicep" Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
e7febf7 to
ef1acec
Compare
Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
ef1acec to
cedafa1
Compare
|
Just rebased - @brooke-hamilton, when you have a moment, would you mind approving the workflows? |
Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
|
Build is failing because of #11026 |
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.
@nellshamrell I think we have a few instances of this in the docs and samples as well that would need to be updated :)
https://github.com/search?q=repo%3Aradius-project%2Fsamples%20rad-bicep&type=code
https://github.com/search?q=repo%3Aradius-project%2Fdocs%20rad-bicep&type=code
|
on it! |
|
Done! Pull requests that go along with this one: |
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
Description
This changes the name of the downloaded bicep binary from "rad-bicep" to "bicep"
Type of change
Fixes: #10383
Contributor checklist
Please verify that the PR meets the following requirements, where applicable:
Here is a link to the relevant docs PR radius-project/docs#1597