Skip to content

terraform version: fix output order#25811

Merged
kmoe merged 2 commits intomasterfrom
terraform-version-order
Aug 19, 2020
Merged

terraform version: fix output order#25811
kmoe merged 2 commits intomasterfrom
terraform-version-order

Conversation

@kmoe
Copy link
Copy Markdown
Member

@kmoe kmoe commented Aug 11, 2020

Running terraform version should output the version number, followed by the outdated message if any. This was broken in #25252 and the order was reversed. This PR fixes the order.

Expected behaviour

★ ./terraform version
Terraform v0.13.0-beta1

Your version of Terraform is out of date! The latest version
is 0.13.0. You can update by downloading from https://www.terraform.io/downloads.html

Actual behaviour

★ ./terraform version
Your version of Terraform is out of date! The latest version
is 0.13.0. You can update by downloading from https://www.terraform.io/downloads.html
Terraform v0.13.0-rc1

@kmoe kmoe requested review from a team and jbardin August 11, 2020 22:24
@kmoe kmoe force-pushed the terraform-version-order branch from 41ad7b6 to 25337b7 Compare August 11, 2020 22:27
@kmoe kmoe added the bug label Aug 11, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 11, 2020

Codecov Report

Merging #25811 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
command/version.go 66.66% <100.00%> (+12.23%) ⬆️
terraform/node_resource_plan.go 93.44% <0.00%> (+1.63%) ⬆️

Copy link
Copy Markdown
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

I'd like to see a test added for this, since the regression caused real issues. I haven't dug too deeply but it at a glance it appears that you can set a custom VersionCheckFunc for the version command, which (I would guess) would allow you to coerce the Outdated flag and specify the new version.

@kmoe
Copy link
Copy Markdown
Member Author

kmoe commented Aug 12, 2020

Added test, which fails without 25337b7.

@kmoe kmoe requested a review from mildwonkey August 12, 2020 20:26
Copy link
Copy Markdown
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

Gorgeous, I love the tests - thanks @kmoe !

@ghost
Copy link
Copy Markdown

ghost commented Oct 10, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants