Conversation
Signed-off-by: John Kastner <jkastner@amazon.com>
f3d5e17 to
fa49921
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the Cedar CLI's lib.rs by splitting it into well-organized modules. The large monolithic library file is being restructured into a hierarchical module system with:
- A
command/module containing individual files for each CLI subcommand - A
utils/module containing shared utility functions for reading schemas, policies, requests, and entities
Changes:
- Split
lib.rsinto modular structure withcommand.rsandutils.rsas the main module hubs - Created 19 new command-specific files in
cedar-policy-cli/src/command/directory for each subcommand - Created 5 new utility files in
cedar-policy-cli/src/utils/directory for shared functionality - Maintained all existing functionality without code logic changes; this is purely organizational
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cedar-policy-cli/src/lib.rs | Reduced to 108 lines, now imports from command and utils modules |
| cedar-policy-cli/src/command.rs | New file defining Commands enum and re-exporting from 15 command submodules |
| cedar-policy-cli/src/command/*.rs | New files containing implementation for each CLI command (authorize, evaluate, validate, etc.) |
| cedar-policy-cli/src/utils.rs | New file organizing utility modules and re-exporting helper functions |
| cedar-policy-cli/src/utils/*.rs | New files containing schema, policies, request, entities, and links utilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 72.47% Status: FAILED ❌ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 87.55% Status: PASSED ✅ Details
|
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 72.47% Status: FAILED ❌ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 87.55% Status: PASSED ✅ Details
|
Description of changes
Sorry for the large diff, but this has been bothering me for a while. I hope you can trust that I haven't changed any of the code.
I want to do something similar for
api.rsincedar-policy, but that's a larger project.Issue #, if available
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., changes to the signature of an existing API).cedar-policy(e.g., addition of a new API).cedar-policy.cedar-policy-core,cedar-validator, etc.)I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):cedar-spec, and how you have tested that your updates are correct.)cedar-spec. (Post your PR anyways, and we'll discuss in the comments.)I confirm that
docs.cedarpolicy.com(choose one, and delete the other options):cedar-docs. PRs should be targeted at astaging-X.Ybranch, notmain.)