Conversation
…y-again Enable Azure Login action by committing compiled output
Co-authored-by: Jury1981 <210622247+Jury1981@users.noreply.github.com>
Co-authored-by: Jury1981 <210622247+Jury1981@users.noreply.github.com>
…pport Co-authored-by: Jury1981 <210622247+Jury1981@users.noreply.github.com>
Co-authored-by: Jury1981 <210622247+Jury1981@users.noreply.github.com>
Reviewer's GuideAdds a dev container-based development environment (docs, helper script, and configuration) and standardizes Azure CLI installation in the canary GitHub Actions workflow by using Flow diagram for dev.sh command routingflowchart TD
A_Start[Start dev.sh] --> B_Get_Command[Read first CLI argument]
B_Get_Command -->|check| C_Check[run_checks]
B_Get_Command -->|build| D_Build[build_action]
B_Get_Command -->|test| E_Test[run_tests]
B_Get_Command -->|dev| F_Dev[dev]
B_Get_Command -->|setup| G_Setup[setup]
B_Get_Command -->|install| H_Install[install_deps]
B_Get_Command -->|clean| I_Clean[clean]
B_Get_Command -->|validate| J_Validate[validate_azure]
B_Get_Command -->|help or none| K_Usage[usage]
B_Get_Command -->|unknown| L_Unknown[log_error Unknown command + usage]
C_Check --> M_End[Exit]
D_Build --> M_End
E_Test --> M_End
F_Dev --> M_End
G_Setup --> M_End
H_Install --> M_End
I_Clean --> M_End
J_Validate --> M_End
K_Usage --> M_End
L_Unknown --> M_End
subgraph Internal_Flows
F_Dev --> D_Build
F_Dev --> E_Test
G_Setup --> H_Install
G_Setup --> D_Build
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
.devcontainer/dev.sh,check_azure_clicallsaz version --output tsv, butaz versiononly supports JSON output; consider parsing the JSON (e.g., withjq) or using a simpleraz versioncall to avoid relying on unsupported--outputand brittlegrepparsing. - The
cleancommand in.devcontainer/dev.shremovesnode_modules/as well aslib/; you might want to separate dependency cleanup from build-artifact cleanup socleandoes not unexpectedly drop installed dependencies for every use. dev.shusesgrep -oP(PCRE) incheck_azure_cli, which will fail on systems with non-GNUgrep(e.g., macOS); consider a more portable approach or guarding this with a GNU-grep check if you expect the script to be run outside the dev container.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `.devcontainer/dev.sh`, `check_azure_cli` calls `az version --output tsv`, but `az version` only supports JSON output; consider parsing the JSON (e.g., with `jq`) or using a simpler `az version` call to avoid relying on unsupported `--output` and brittle `grep` parsing.
- The `clean` command in `.devcontainer/dev.sh` removes `node_modules/` as well as `lib/`; you might want to separate dependency cleanup from build-artifact cleanup so `clean` does not unexpectedly drop installed dependencies for every use.
- `dev.sh` uses `grep -oP` (PCRE) in `check_azure_cli`, which will fail on systems with non-GNU `grep` (e.g., macOS); consider a more portable approach or guarding this with a GNU-grep check if you expect the script to be run outside the dev container.
## Individual Comments
### Comment 1
<location path=".devcontainer/README.md" line_range="9" />
<code_context>
+
+- **Node.js 20** - JavaScript/TypeScript runtime
+- **Azure CLI** - Command-line tools for Azure
+- **PowerShell** - Cross-platform PowerShell for Azure PowerShell module
+- **GitHub CLI** - Command-line tools for GitHub
+- **GNU Core Utilities** - Standard Unix tools (bash, grep, sed, etc.)
</code_context>
<issue_to_address>
**suggestion (typo):** Consider using the plural "modules" for Azure PowerShell here.
This keeps the terminology consistent with your later reference to "Azure PowerShell modules."
```suggestion
- **PowerShell** - Cross-platform PowerShell for Azure PowerShell modules
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| - **Node.js 20** - JavaScript/TypeScript runtime | ||
| - **Azure CLI** - Command-line tools for Azure | ||
| - **PowerShell** - Cross-platform PowerShell for Azure PowerShell module |
There was a problem hiding this comment.
suggestion (typo): Consider using the plural "modules" for Azure PowerShell here.
This keeps the terminology consistent with your later reference to "Azure PowerShell modules."
Suggested change
| - **PowerShell** - Cross-platform PowerShell for Azure PowerShell module | |
| - **PowerShell** - Cross-platform PowerShell for Azure PowerShell modules |
|
This PR is idle because it has been open for 14 days with no activity. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Create security detail for new sponsored establishments to the IE and the Kingdom of Athens Based in Berkeley, California and MIT. It will be sponsorship for our client and conglomerate of highly qualified ai security officers through Federal HCS Programs through ```
``