PR to add Thiserror to cfn-guard #329
Merged
Conversation
This file contains 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
akshayrane
requested changes
Feb 9, 2023
joshfried-aws
requested review from
akshayrane,
razcloud,
Brianwithay21,
wkexiang,
reza-aws and
Neel-Renavikar
Brianwithay21
approved these changes
Feb 14, 2023
joshfried-aws
force-pushed
the
thiserror
branch
from
956f2d8
to
a2b2204
Compare
akshayrane
requested changes
Feb 14, 2023
| @@ -8,10 +8,12 @@ mod rules; | |||
| mod utils; | |||
|
|
|||
| use crate::commands::{MIGRATE, OUTPUT, PARSE_TREE, RULEGEN}; | |||
| use crate::utils::writer::WriteBuffer::Stderr; | |||
| @@ -62,7 +64,10 @@ fn main() -> Result<(), Error> { | |||
|
|
|||
There was a problem hiding this comment.
Should be somewhere before this block, we initialize Stdout to be the err buffer, right?
| #[case] rules_arg: &str, | ||
| #[case] expected_writer_output: &str, | ||
| #[case] expected_status_code: i32, | ||
| ) { | ||
| let mut writer = Writer::new(WBVec(vec![])); | ||
| let mut writer = Writer::new_with_custom_stderr(WBVec(vec![])); |
There was a problem hiding this comment.
I thought we were going to remove these custom constructors to replace it with a generic one all across the code base, no?
akshayrane
approved these changes
Feb 14, 2023
joshfried-aws
added a commit
that referenced
this pull request
Mar 23, 2023
* Improve handling of function references for test command (#331) * improving handling for tagged values for test command * removing shorthand functions from known issues * PR to add Thiserror to cfn-guard (#329) * PR to add thiserror to guard * cleanup * clippy lints * printing errs to stderr instead of stdout * merge conflicts * rustfmt * cleaning up code * Redirected verbose output from stdout to custom writer and added unit… (#332) * Redirected verbose output from stdout to custom writer and added unit tests * Updated one more occurrence & removed new line from 2 places --------- Co-authored-by: Akshay Rane <raneaks@amazon.com> * Addit cargo-audit to CI + bump up clap to 3.0 (#330) * init commit, bumped up clap to 3.0 * cargo audit for CI * reverting last commit + changing cargo-audit to not be part of build step, only ran every night * changing cronjob to run at noon instead of midnight * fixing error in cronjob scheduling * Implemented custom reader, increasing test coverage for validate command. (#334) * init commit for reader + more unit tests for validate * init commit for reader + more unit tests for validate * temp * temp2 * rebasing * clippy lints for validate * more tests for validate * adding thiserror license to attributions * removing useless code * renaming tests * fix failing test * fixes as per comments * attribution update * moved get_reader fn to utils * Update CONTRIBUTING.md (#335) * Clap4 (#336) * init commit, bumped up clap to 3.0 * cargo audit for CI * reverting last commit + changing cargo-audit to not be part of build step, only ran every night * init commit bumping up to clap4 * cleaning some code up * temp commit * rebasing + fixing small bug * cleanup * adding help messages * adding more tests for test_command * more tests for prev engine * improving code style * typo * fixed unecessary match * cleaning up test command * fixes as per comments * fixes as per comments * fixed failing build * Added integration tests against aws-guard-rules-registry on Ubuntu (#337) * Added GitHub action for integration test with rules registry * Changed branch name for testing * Changed ref tag * Changed repo param * Removed tags temporarily * Added temp in-place replacement and main branch as ref * Corrected the sequence of commands * Added parse-tree integration tests * Changed dir and limited build to one crate * Added names to each job * Added logic to allow capturing of exit codes * Testing exit code script * Added if as part of error handling * Added statements for test command * Version before changing branch * Changed branches * Bug fix for output arg being pulled before we enter command context * Workaround to skip comments only files * Added test remote branch back * Corrected the condition * Made SKIPPED_RULE_COUNT a variable * Updated display messages * Removed tabs * Updated branches * Updated code to extract OUTPUT arg from subcommand instead of the app --------- Co-authored-by: Akshay Rane <raneaks@amazon.com> * Update check-tags-present.guard (#313) replaced "is" with "if". Also. can you provide more detailed explanation for this guard file * Adding structured evaluator (#339) * init commit for reader + more unit tests for validate * implemented structured reporter + some small refactorings * clippy lints * adding structured reporter for payload code path * adding test for structured payload * cleanup * adding previoius-engine as a conflict with structured * adding fix for recursive serialization issue * adding test for structured yaml * Added deprecated short flag for print-json in parse-tree (#345) * Added deprecated short flag for print-json in parse-tree * Formatting changes * Added docs --------- Co-authored-by: Akshay Rane <raneaks@amazon.com> --------- Co-authored-by: Akshay Rane <aks.rane@gmail.com> Co-authored-by: Akshay Rane <raneaks@amazon.com> Co-authored-by: swiercek <111157886+swiercek@users.noreply.github.com> Co-authored-by: Aishwarya4400 <50081627+Aishwarya4400@users.noreply.github.com>
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.
Issue #, if available:
Description of changes:
This pr changes the way we handle errors to use the popular rust crate called thiserror. This helps us reduce nesting for enum variants, allowing for more readable code. This change also makes it easier to integrate with other crates in the future.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license