Skip to content
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 formatting round trip - Style is always 0 #243

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

ericzbeard
Copy link
Contributor

@ericzbeard ericzbeard commented Jan 18, 2024

When formatting to JSON and back to YAML, JSON styles were left in the YAML nodes, which caused inconsistent formatting.

This might be considered a breaking change, because one of the effects is to now format !Sub without double quotes, even if they were in the original.

Fixes #170

@ericzbeard
Copy link
Contributor Author

Can't add you as an official review but please take a look if you have time @hariprakash-j @iainelder

@hariprakash-j
Copy link
Collaborator

@ericzbeard , is there any reason why the input yaml files are inline and in its own file ? if for some reason it needs to be changed would it not be confusing to change the same thing in two places? Wouldn't it be better to read the file and remove the inline to keep the data in one place.

@hariprakash-j
Copy link
Collaborator

hariprakash-j commented Jan 19, 2024

just a small thing, in the file format_test.go the function checkMatch has the input var coded inside the function instead of it being an arg to the function, passing in the input as an arg would make it more reusable if some one wants to use it later with a different input.

@ericzbeard
Copy link
Contributor Author

@ericzbeard , is there any reason why the input yaml files are inline and in its own file ? if for some reason it needs to be changed would it not be confusing to change the same thing in two places? Wouldn't it be better to read the file and remove the inline to keep the data in one place.

I only added the separate file so I could see the output directly from the command, outside of unit tests. The file isn't actually being used for anything else.

@ericzbeard
Copy link
Contributor Author

just a small thing, in the file format_test.go the function checkMatch has the input var coded inside the function instead of it being an arg to the function, passing in the input as an arg would make it more reusable if some one wants to use it later with a different input.

Yep, I'll change that if I need to add any more tests in the future.

@hariprakash-j
Copy link
Collaborator

everything its okay, just noticed those two things 👍

@ericzbeard ericzbeard merged commit 5764cf3 into aws-cloudformation:main Jan 22, 2024
1 check passed
@ericzbeard ericzbeard deleted the fix-170 branch January 22, 2024 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rain fmt doesn't do round trip
2 participants