-
Notifications
You must be signed in to change notification settings - Fork 19
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
chore: remove dependency on serde_json
#102
Conversation
25ec771
to
4d70681
Compare
huh...didn't think about it that yaml is a superset (structure-wise). So true. |
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.
Most of my stuff are nitpicks, but dunno about removing !Ref/!Condition. Take a look at those fellas.
) | ||
.get_matches(); | ||
|
||
if matches.is_present("inputFormat") { | ||
eprintln!("--inputFormat (-f) is a no-op and will be removed in a future version. All input is treated as YAML"); | ||
eprintln!("as it is a strict super-set of JSON (all valid JSON is valid YAML)."); |
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.
Note: YAML was added last month, and isn't actually published in a new release. Not sure we need the deprecation notice, but it is a good exercise.
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.
I think this works for both json and yaml if I'm not mistaken
src/parser/condition.rs
Outdated
"!Condition" | "Condition" => { | ||
Some("Condition") => { |
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 sample here says that !Condition
is real:
CreateBucketPolicy: !And
- !Condition IsProduction
- !Condition CreateBucket
src/parser/resource.rs
Outdated
"!Ref" | "Ref" => { | ||
Some("Ref") => { |
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.
I believe !Ref is legit as well: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-ref.html#ref-example
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.
Ah yes... I made a boo-boo here... Because I changed approaches in the middle of doing this and since those aren't Fn::
intrinsics I forgot about them...
src/parser/condition.rs
Outdated
} | ||
"!Ref" | "Ref" => { | ||
Some("Ref") => { |
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.
09e7389
to
64c224a
Compare
64c224a
to
2d6a687
Compare
had to fix some merge conflicts so I messed with the branch a bit. All good now |
Since all valid JSON is also valid YAML, the
serde_yaml
library can be used to parse all kinds of input, without requiring user selection. Also changes the initial internal representation to be a YAML value instead of a JSON value, which is a little more expressive (although it's not clear whether this is a good thing in this particular case; but at least it reduces the dependency surface).