-
Notifications
You must be signed in to change notification settings - Fork 183
TFC: Implement run panel for viewing plan #1590
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
Conversation
9a8d236
to
589e88b
Compare
2de7fc6
to
9d51fc0
Compare
0e55ed9
to
6c7dadc
Compare
08a6864
to
ac777a1
Compare
ac777a1
to
77666a6
Compare
07a2105
to
bec481d
Compare
bec481d
to
be4e696
Compare
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.
Works great!
I added a couple of questions and minor refactoring suggestions.
be4e696
to
d040c09
Compare
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.
Thank you for taking the time to implement my suggestions.
LGTM 👍
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Closes #1506
The issue covers both plan and apply, so it's not fully being fixed by this PR yet but we track apply in a follow-up issue #1637
I went as far in the implementation as I felt was reasonable in terms of effort/LOC and value added. Some trade-offs/TODOs are listed below, which I'm happy to file if we agree on those.
The exclusion of testing in particular is a little unfortunate because there's no simple way of telling the user this is a missing feature. They just won't see any tree items and instead see the same welcome view. It's not great but I'm really struggling to come up with a good way around it and I'd greatly appreciate input on this. Also there's so many different test messages with different contexts that it feels like adding support would be quite involved.
Similarly, nesting is a bit more complex than I originally thought, because once we nest under e.g. module, or even just resource type, then we suddenly face situation where we have to either "invent" new icons for the mix of actions (create/update/delete/...) or risk reducing the usefulness of the panel if we don't put any icons there.
UX
Summary: This adds a panel to render the plan log which was previously rendered as raw JSON.
JSON (prior to this PR)
Planned changes
Outputs
Diagnostics
Drifts
Fallback for CLI (non-structured) output
TODO