-
Notifications
You must be signed in to change notification settings - Fork 327
Conversation
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.
Thanks for adding this flag to our fmt
command! I've got a small doc string suggestion for you, but otherwise this makes sense!
Suggestion makes sense to me, I almost went for that initially but opted for the language in the issue instead. I think this is definitely clearer. 👍 |
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.
Looks great to me! Tested it locally and it works.
I went ahead and made a changelog and auto-generated the website for you, however since this PR was opened on your fork, I don't think I have push permissions. If you wanted to cherry-pick my commits then I'd say we're good to merge this one! It's over on this branch: https://github.com/hashicorp/waypoint/compare/4020/check-flag-fmt?expand=1
Co-authored-by: Brian Cain <bcain@hashicorp.com>
8f959f1
to
92ae01d
Compare
Just rebased main on and then cherry picked your commits. Did em in reverse order so changelog was last but I assume it's getting squashed anyway so prolly doesn't matter. Thanks! |
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.
Looks good to me! Thank you for making this contribution! 🎉
This PR is meant to handle #3817 and add check support to the fmt command. There's not much to it but in local testing with waypoint.hcl files it seemed to perform as expected, though it's kinda funny that the default config is generated needing formatting.