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(output): Remove Refreshing state... from output #1352

Merged
merged 2 commits into from
Feb 25, 2021

Conversation

mathcantin
Copy link
Contributor

Fixes #1306

Since Terraform 0.14.0 there are no separator between refreshing plan and the plan. This strategy is to find the last line with "Refreshing state..." and remove it with all of the above.

I remove the sentence "Refreshing Terraform state in-memory prior to plan..." from tests because Terraform don't show that anymore.

Since Terraform 0.14.0 there are no separator between refreshing plan and the plan.
@mathcantin mathcantin force-pushed the remove-refreshing-state branch from a6e8f37 to 7a8a236 Compare February 2, 2021 03:34
@mathcantin
Copy link
Contributor Author

I update the PR to check if Terraform is in version >= 0.14.0.

@mathcantin mathcantin requested a review from lkysow February 2, 2021 03:35
@mathcantin
Copy link
Contributor Author

@lkysow Do you have time to check this?

@nishkrishnan nishkrishnan added waiting-on-review Waiting for a review from a maintainer and removed waiting-on-response Waiting for a response from the user labels Feb 8, 2021
@bryantbiggs
Copy link
Contributor

this would be greatly appreciated - we upgraded to TF 0.14.x to get the concise diffs but now we have a wall of Terraform refresh text output 🙏🏼

@Ninir
Copy link
Contributor

Ninir commented Feb 17, 2021

@chenrui333 Hi! would it be possible to get your eyes on it? we'd love to help in any way, and appreciate any time you might spend on this. Thank you!

@mathcantin
Copy link
Contributor Author

@nishkrishnan Could you check this?

Copy link
Contributor

@nishkrishnan nishkrishnan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good, I see that you've been submitting a bunch of PRs around 0.14.0 improvements and thanks for your contribution. But in these cases where there are lot of if and elses being done to preserve backwards compatibility I'd suggest something along the lines of using composition to abstract the underlying implementation and wrapping them up in a nicely packaged interface so we can proxy to implementations based on a condition. Will make things more readable and easy to add changes for future versions.

server/events/runtime/apply_step_runner.go Outdated Show resolved Hide resolved
@nishkrishnan nishkrishnan removed the waiting-on-review Waiting for a review from a maintainer label Feb 25, 2021
@nishkrishnan nishkrishnan merged commit 7301feb into runatlantis:master Feb 25, 2021
Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 lgtm

@Ninir
Copy link
Contributor

Ninir commented Feb 25, 2021

Thanks @chenrui333 and @nishkrishnan, definitely appreciate it! 🙇

@nishkrishnan
Copy link
Contributor

nishkrishnan commented Feb 25, 2021

you're gonna need to update test fixtures and then I can merge it back in

@mathcantin
Copy link
Contributor Author

you're gonna need to update test fixtures and then I can merge it back in

What?

@nishkrishnan
Copy link
Contributor

The tests failed on master so I'm reverting the merge.

@mathcantin
Copy link
Contributor Author

mathcantin commented Feb 25, 2021

I'm sorry, that the line 101, my editor remove the last space automatically. I fix that.

@mathcantin
Copy link
Contributor Author

I'm reopen a new PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants