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

Sort unmanaged & deleted resources in a predictable order #356

Merged
merged 8 commits into from
Mar 23, 2021

Conversation

sundowndev
Copy link
Contributor

Q A
πŸ› Bug fix? yes
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #321
❓ Documentation no

Description

For now, resources were fetched in parallel which would cause the resource list order to be unpredictable. With these changes, the resources are always sorted by their Terraform Id.

@sundowndev sundowndev added kind/enhancement New feature or improvement priority/3 labels Mar 19, 2021
@sundowndev sundowndev added this to the v0.8.0 milestone Mar 19, 2021
@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #356 (06f7d4f) into main (f34d544) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #356      +/-   ##
==========================================
+ Coverage   69.44%   69.48%   +0.04%     
==========================================
  Files         281      281              
  Lines        6016     6024       +8     
==========================================
+ Hits         4178     4186       +8     
  Misses       1486     1486              
  Partials      352      352              
Impacted Files Coverage Ξ”
pkg/analyser/analysis.go 96.96% <100.00%> (+0.09%) ⬆️
pkg/analyser/analyzer.go 92.10% <100.00%> (+0.10%) ⬆️
pkg/resource/resource.go 85.71% <100.00%> (+7.93%) ⬆️

lotoussa
lotoussa previously approved these changes Mar 19, 2021
Copy link
Contributor

@lotoussa lotoussa left a comment

Choose a reason for hiding this comment

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

lgtm rn

pkg/analyser/analyzer.go Show resolved Hide resolved
pkg/analyser/analyzer_test.go Outdated Show resolved Hide resolved
pkg/resource/resource.go Outdated Show resolved Hide resolved
@sundowndev sundowndev marked this pull request as ready for review March 22, 2021 10:23
@sundowndev sundowndev requested a review from a team as a code owner March 22, 2021 10:23
moadibfr
moadibfr previously approved these changes Mar 22, 2021
Copy link
Contributor

@moadibfr moadibfr left a comment

Choose a reason for hiding this comment

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

LGTM

@sundowndev sundowndev changed the title Sort resources in a predictable order Sort unmanaged & deleted resources in a predictable order Mar 22, 2021
Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

I'm fine with that

@eliecharra eliecharra added this to Review in driftctl via automation Mar 23, 2021
@eliecharra eliecharra merged commit 82f74f7 into main Mar 23, 2021
driftctl automation moved this from Review to Done Mar 23, 2021
@eliecharra eliecharra deleted the refact/sortResources branch March 23, 2021 10:23
@eliecharra eliecharra linked an issue Mar 23, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or improvement priority/3
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Sort resource names in unmanaged resources list
5 participants