8000 Split aws_iam_role_policy_attachment by eliecharra · Pull Request #839 · snyk/driftctl · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Split aws_iam_role_policy_attachment #839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue 8000 and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 13, 2021
Merged

Split aws_iam_role_policy_attachment #839

merged 1 commit into from
Jul 13, 2021

Conversation

eliecharra
Copy link
Contributor

Fix #724

@eliecharra eliecharra added the kind/maintenance Refactoring or changes to the workspace label Jul 13, 2021
@eliecharra eliecharra added this to the Deep Mode milestone Jul 13, 2021
@eliecharra eliecharra requested a review from a team July 13, 2021 09:17
@codecov
Copy link
codecov bot commented Jul 13, 2021 •
edited
Loading

Codecov Report

Merging #839 (f2e9542) into main (8909d15) will increase coverage by 0.09%.
The diff coverage is 85.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #839      +/-   ##
==========================================
+ Coverage   81.32%   81.41%   +0.09%     
==========================================
  Files         226      227       +1     
  Lines        7340     7355      +15     
==========================================
+ Hits         5969     5988      +19     
+ Misses       1153     1148       -5     
- Partials      218      219       +1
Impacted Files Coverage Δ
pkg/remote/aws/init.go 0.00% <0.00%> (ø)
.../aws/iam_role_policy_attachment_details_fetcher.go 78.94% <78.94%> (ø)
...emote/aws/iam_role_policy_attachment_enumerator.go 94.11% <94.11%> (ø)

lotoussa
lotoussa previously approved these changes Jul 13, 2021
View reviewed changes
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

sundowndev
sundowndev reviewed Jul 13, 2021
View reviewed changes
pkg/remote/aws/iam_role_policy_attachment_details_fetcher.go

func (r *IamRolePolicyAttachmentDetailsFetcher) ReadDetails(res resource.Resource) (resource.Resource, error) {
ctyVal, err := r.reader.ReadResource(terraform.ReadResourceArgs{
Ty: aws.AwsIamRolePolicyAttachmentResourceType,
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we already did that in every other details fetchers, but is there a reason to not use the resource type defined by the enumerator ? We could just call res.TerraformType() here

sundowndev
sundowndev previously approved these changes Jul 13, 2021
View reviewed changes
@lotoussa lotoussa dismissed stale reviews from sundowndev and themself via f2e9542 July 13, 2021 12:54
@lotoussa lotoussa force-pushed the rework_iam_role_policy_attachment branch from 5fee8fe to f2e9542 Compare July 13, 2021 12:54
lotoussa
lotoussa approved these changes Jul 13, 2021
View reviewed changes
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.

rebase main, approving review.

@lotoussa lotoussa merged commit 8baac05 into main Jul 13, 2021
@lotoussa lotoussa deleted the rework_iam_role_policy_attachment branch July 13, 2021 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lotoussa lotoussa lotoussa approved these changes

@sundowndev sundowndev sundowndev left review comments

Assignees
No one assigned
Labels
kind/maintenance Refactoring or changes to the workspace
Projects
None yet
Milestone
Deep Mode
Development

Successfully merging this pull request may close these issues.

Split iam_role_policy_attachment_supplier
3 participants
@eliecharra @sundowndev @lotoussa
0